Skip to content

BLD/WHL: declare Python 3 wheels as abi3-compliant#151

Draft
neutrinoceros wants to merge 1 commit intobrandon-rhodes:masterfrom
neutrinoceros:whl/abi3
Draft

BLD/WHL: declare Python 3 wheels as abi3-compliant#151
neutrinoceros wants to merge 1 commit intobrandon-rhodes:masterfrom
neutrinoceros:whl/abi3

Conversation

@neutrinoceros
Copy link

this may overlap partially with #150 (I'll notify the author there)
In short, this makes it so that all supported Python 3 versions (including future ones) are served with a single wheel (per platform). The tradeoff with this technique is usually that some faster C APIs may not be available, but it looks like the extension here isn't relying on any of those, so it's actually free here.

@neutrinoceros neutrinoceros changed the title BLD/WHL: declare wheels as abi3-compliants BLD/WHL: declare Python 2 wheels as abi3-compliants Mar 8, 2026
@neutrinoceros neutrinoceros changed the title BLD/WHL: declare Python 2 wheels as abi3-compliants BLD/WHL: declare Python 2 wheels as abi3-compliant Mar 8, 2026
@neutrinoceros neutrinoceros force-pushed the whl/abi3 branch 2 times, most recently from 668ce11 to 22d3d1d Compare March 8, 2026 18:30
@neutrinoceros neutrinoceros marked this pull request as ready for review March 8, 2026 18:30
@neutrinoceros neutrinoceros changed the title BLD/WHL: declare Python 2 wheels as abi3-compliant BLD/WHL: declare Python 3 wheels as abi3-compliant Mar 8, 2026
@@ -1,3 +1,6 @@
// keep in sync with setup.py setup(options=...)
#define Py_LIMITED_API 0x030800f0
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that I didn't make this conditional because (supposedly) Python 2 isn't even aware of this pre-proc def so it shouldn't choke on it

(https://peps.python.org/pep-0384/)

setup.py Outdated
provides = ['sgp4'],
ext_modules = ext_modules,
# keep in sync with wrapper.cpp Py_LIMITED_API
options={"bdist_wheel": {"py_limited_api": f"cp38"}} if sys.version_info >= (3, 8) else {},
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

however, I can't tell for sure what setuptools does when passed this option on Python 2 so I opted for a safer approach

@@ -1,3 +1,8 @@
#ifndef
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you need to name the macro on the #ifndef line?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of course. My C is a bit rusty, but in hindsight I'm mostly surprised to see this compiled in the first place... anyway, fixed now !

@neutrinoceros
Copy link
Author

@brandon-rhodes, before I dive in and try to fix the failing build on macos, I noticed that the build matrix only started at Python 3.10, not 3.8 as I assumed. Is this expected ? I can easily adapt this to be dynamic on the version that's actually used to build, which should be more flexible.

@neutrinoceros
Copy link
Author

also: do you intend to keep compat with 2.7 ?

@brandon-rhodes
Copy link
Owner

before I dive in and try to fix the failing build on macos, I noticed that the build matrix only started at Python 3.10, not 3.8 as I assumed. Is this expected ?

I vaguely assumed that, when @mworion’s recent PR omitted 3.8 and 3.9, it was because the tooling was no longer available on CI to get them easily built. So it was not "expected" in the sense that I could have told you ahead of time that those two would be dropped; but, "expected" in the sense that Python tools seem to be dropping old versions and abandoning legacy users at an alarming rate, so I'm not surprised when contributer PRs drop them.

Also, sgp4 is not really changing from one version to the next, so I assumed that 3.8 folks could just keep using the old wheels without much problem?

also: do you intend to keep compat with 2.7 ?

If it's easy to do so, then, yes, it's nice to keep any remaining 2.7 folks supported. But if it's awkward and hard to maintain, then they can probably keep happily using a previous version of sgp4, since we're not adding new features at this point?

@neutrinoceros
Copy link
Author

thanks for your quick answers. I tried rewriting this in a version agnostic fashion an preserve compat for Python 2, so now the infrastructure is the one source of truth for which versions should be built.
As long as Python versions are built for in increasing order, cibuildwheel should be smart enough to recognize when a previous build can simply be re-used and tested.

@neutrinoceros neutrinoceros marked this pull request as draft March 10, 2026 21:07
@neutrinoceros
Copy link
Author

neutrinoceros commented Mar 10, 2026

switching to draft as I only now realize that the build is actually failing eventhough it completes "successfully"

7 errors generated.

I missed that I should have been using PYTHON_SGP4_COMPILE=always.
I think all of the errors I'm getting are fixable, but it'll definitely be more work, so don't hold your breath !

@brandon-rhodes
Copy link
Owner

Thanks for double-checking, and for taking another look at this. (I will soon embark on a quick vacation, so if I don't respond immediately when you circle back, just wait and I should return soon.)

@neutrinoceros
Copy link
Author

neutrinoceros commented Mar 11, 2026

I've opened the following PRs in support of this patch:

the latter 3 should represent the entire set of changes required, and are all based of the first one so CI would fail if anything isn't working.
At the time of writing, #157 is the only part that does not compile locally for me yet.

@neutrinoceros
Copy link
Author

FWIW I successfully ran the test suite locally with all the preliminary PRs + this one combined locally, but I do think we should take care of #158 before merging anything beyond #153

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants