BLD: reintroduce compatibility shim for compiling against numpy 1.x #147
Conversation
There was a problem hiding this comment.
OK, makes sense - there is no reason to, effectively, forbid building against earlier numpy; I think this part really teaches a lesson that if we support earlier numpy, we should also support building against it. Indeed, we may want to add a test in our CI that ensures this actually works. Is that easy to add? If not, I'm happy to just merge this now.
|
p.s. It is not directly related, but I do feel that there now is even more reason not to enforce minimum versions in |
In fact, don't we have to remove the constraint in |
not if they skip build-isolation, which is the workflow that I'm trying to fix here :) |
Sorry I missed that. I think it may be easy, I'll give it a try now. |
neutrinoceros
force-pushed
the
bld/numpy_1.x_compat_shim
branch
from
898174f
to
c0cf6fb
Compare
|
I just force-pushed with just the new CI job so we can see it failing first. |
neutrinoceros
force-pushed
the
bld/numpy_1.x_compat_shim
branch
2 times, most recently
from
fd1ebb4
to
68beb3a
Compare
|
(of course it needs to fail for the expected reason. I'll iterate until I get it right) |
neutrinoceros
force-pushed
the
bld/numpy_1.x_compat_shim
branch
3 times, most recently
from
af7814b
to
96d34c0
Compare
neutrinoceros
force-pushed
the
bld/numpy_1.x_compat_shim
branch
from
96d34c0
to
0b758b5
Compare
| python -m pip install --upgrade wheel setuptools setuptools_scm jinja2 'numpy<2.0' | ||
| - run: python -m pip list | ||
| - name: Build | ||
| # using --no-build-isolation allows us to skip pass build-system metadata |
There was a problem hiding this comment.
It still seems to me we're papering over the main problem, that this all comes about because we have the requirement of numpy>=2.0.0rc1 in pyproject.toml. At least, it looks to me that, as is, a pip install pyerfa on the astropy side for s390 etc will still force installation of numpy 2.0 to produce the pyerfa binaries and thus fail (unless we add --no-build-isolation there too, but why should astropy have to worry about this?).
There was a problem hiding this comment.
I guess to put it a bit more constructively, I think this test should verify that pip install pyerfa is possible on a system that does not have numpy 2.0 (and cannot build it either).
There was a problem hiding this comment.
The way I see it, it's only a trade-off for where we want things to be simple and where we accept that they get more complicated.
Having numpy>=2.0.0rc1 gives a behavior that's almost always desirable, and crucially, doesn't prevent working with numpy 1.x when needed. Whereas not having it moves the complexity to something much more crucial: the releasing process. I personally feel it's preferable to put the extra workload on rare archs.
There was a problem hiding this comment.
My problem is that we're making problems on rare archs outside of our own package - I dislike that astropy has to work around a choice of convenience made in pyerfa.
I also generally dislike that we effectively state falsehoods: numpy>=2.0.0.rc1 is in fact not required to build pyerfa; it is just something that's there because it can help create a package that is useful in more circumstances (which for a general pip install is likely irrelevant).
Anyway, this is slightly orthogonal to the issue at hand, which really just reflects that pip is not a great package manager (and that creating a good package manager is incredibly hard). Let's first of all make sure astropy can build at all!
There was a problem hiding this comment.
Let's first of all make sure astropy can build at all!
To be clear, astropy has no build issue. What's broken is building pyerfa on rare arch's without numpy 2, and it happens that this is only tested in the astropy repo
There was a problem hiding this comment.
What's broken is building pyerfa on rare arch's without numpy 2
Indeed, and my suggestion was mostly meant to ensure that on the astropy side one doesn't have to give any special flags to get pyerfa to build properly when numpy 2.0 is not available.
But obviously without reintroducing the *elsize* stuff that will not be possible at all, so let's get that in first!
There was a problem hiding this comment.
Ready when you are :)
|
OK, let's get this in so we can unblock astropy. We can discuss the build requirements elsewhere. |
|
@mhvk just to make sure we're on the same page; a release with this patch would be needed to actually unblock astropy's weekly cron. |
|
A release would indeed by nice so we can resolve: Thanks, all! |
|
OK, I pushed a changelog entry and new tag, so the next version is being created: https://github.com/liberfa/pyerfa/actions/runs/8664641913 |
|
Thanks! I see it on PyPI. Should we yank 2.0.1.3 too? 🤷♀️ |
|
I triggered the exotic archs job for astropy. I'll let you know in a few hours. 😆 🤞 |
|
Ooops sorry, looks like they don't listen to workflow_dispatch for some reason (PR for another time) so I am rerunning the scheduled one here instead: https://github.com/astropy/astropy/actions/runs/8595346105 |
Not sure what is best practice, but the wheels for 2.0.1.3 did work properly, for all supported versions of numpy, so my sense is that there is no need to yank. But let's ping @astrofrog. |
Hmm. Then why did it crash for 3 of the 4 "exotic" archs over at astropy? 🤔 |
|
Only building from source against numpy 1.x was broken, I don't think it warrants a yank, especially now that there's a new latest release. |
|
Okay then, thanks for the clarification! |
|
main passed now https://github.com/astropy/astropy/actions/runs/8595346105 v6.1.x also passed https://github.com/astropy/astropy/actions/runs/8663207597 Thanks for the quick fix and release! |
This reverts a cleanup I did rather eagerly in #144, and re-introduces a compatibility shim originally added in #140
Rationale: even if we're now requiring numpy>2.0.0rc in pyproject.toml, this bit of meta data is ignored by pip when installing with
--no-build-isolation. On old and exotic architectures, where numpy 2.0.0rc1 isn't available, it's not problematic to build against 1.x instead, and can save the (long) time required to build numpy itself (if it even works).In short, this would replace astropy/astropy#16284