Repository navigation
Going forward after numpy 2.0's change of copy semantics #16167
Description
This issue is to track and discuss the follow up of #16142
Here's the context in a nutshell:
- up to now astropy has been following numpy's semantics regarding the meaning of the
copykeyword argument innumpy.array's signature - numpy 2.0 changes this semantic (see API: Introduce
copyargument fornp.asarray[Array API] numpy/numpy#25168)
Here's a summary of old/new behaviors
copy |
numpy 1.x | numpy 2.0 |
|---|---|---|
True |
always copy | always copy (unchanged) |
False |
copy if and only if it cannot be avoided | do not perform a copy, raise an error if it cannot be avoided |
None |
ValueError |
copy if and only if it cannot be avoided (same a False in numpy 1.x) |
It follows that we cannot keep both the following implicit, and now conflicting, promises
- future astropy APIs (e.g.
Quantity(..., copy=False)) will always match their past behavior - astropy APIs follow numpy's
So we need to choose which one we care about most, and apply necessary transformations in that direction.
Another, equivalent, way to think about this choice is whether, going forward, we should support numpy 1 semantics or numpy 2 semantics.
I'm personally inclined to think 2) (follow numpy 2.0) should be preferred because
- the new numpy API offers more flexibility to users (or in other words, numpy 1.x is too restrictive)
- at some point in the future, no one will be running numpy 1.x any more so matching that API is guaranteed to surprise some users
So, assuming my preference is also consensual, let me draft out a plan for this pivoting. In the following,
I'll be referring to compatibility shims introduced in #16142 as "band aids"
- make the band aids redundant by refactoring our internal usage of
numpy.array(..., copy=False)(this was originally part of BUG: fix compatibility with numpy 2.0 copy semantics #16142, and is re-rolled in RFC: refactor internal usage of copy=False to copy=COPY_IF_NEEDED #16166). I think this part is probably non-controversial as long as we don't attempt to backport it, so as it stands, it a pure refactor for astropy 6.1.0. This is needed in preparation to step 2 (avoid self-triggered warnings) - deprecate band aids: warn users that
copy=Falsewill raise an error in the future if a copy cannot be avoided. The details of this deprecation will require further discussion1, but I think we can start working on it as soon as RFC: refactor internal usage of copy=False to copy=COPY_IF_NEEDED #16166 is dealt with. - eventually, remove all band aids (this would be a major API change, so it can only happen in a x.0 release ?)
This discussion originated in #16142 (review) and #16142 (comment)
Footnotes
-
when should it end ? do we want to warn all users or just the ones that are already on numpy >= 2.0 ? should we support some way to opt-in future behavior, and how ? ↩
Activity
pllim commentedon Mar 6, 2024
Would it help to put this on the dev telecon agenda?
neutrinoceros commentedon Mar 6, 2024
Yes, please ! I didn't think of that, thank you !
pllim commentedon Mar 7, 2024
For the record, from 2024-03-07 dev telecon notes:
astropyshall match whatever the installednumpyversion does. Avoid work to put in and later remove again compatibility layers, configuration options etc. Clément working on this.astropy-devmailing list when ready, so stakeholders like downstream pipeline maintainers can test it out to ensure no surprises later when numpy and/or astropy are released.neutrinoceros commentedon Mar 8, 2024
Following that discussion, I've prepared two alternative PRs for downstream testing:
both PRs share consensual ancestors that are pure refactors but need to be addressed first
neutrinoceros commentedon Mar 11, 2024
I just sent out an email to astropy-dev to call for feedback on #16181
neutrinoceros commentedon Mar 15, 2024
It was suggested that maybe we should backport this family of patches to astropy 6.0.x
see #16181 (comment)
At this point, the list, in merge order, includes
CylindricalDifferential.__init__fromFalsetoTrue#16198I'm not so sure about #16198 (it's an actual API change), but I'm still considering the idea of backporting all of these manually in one bulk; not backporting them would mean that some of our users would see breakage when they upgrade astropy to 6.1.0. On the other hand, backporting puts the break on the upgrade to numpy 2.0 (where it logically belongs).
neutrinoceros commentedon Mar 15, 2024
⬆️ quoting from @taldcroft's reply in #16181 (comment)
This should now be discussed in the following, broader scoped, issue : #16200
neutrinoceros commentedon Mar 26, 2024
To anyone interested, I've opened a smaller related issue at #16235
pllim commentedon Apr 3, 2024
I think merging #16252 closed this by accident, so re-opening.
What are the remaining things needed to resolve this? Thanks!
neutrinoceros commentedon Apr 3, 2024
#16181 is the last missing piece, I believe