Repository navigation
BUG: fix np.array's dispatcher for compatibility with numpy 2.4 - #18841
Conversation
neutrinoceros
added
Bug
no-changelog-entry-needed
numpy-dev
backport-v7.2.x
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
|
Welcome to Astropy 👋 and congratulations on your first pull request! 🎉 A project member will respond to you as soon as possible; in the meantime, please have a look over the Checklist for Contributed Code and make sure you've addressed as many of the questions there as possible. If you feel that this pull request has not been responded to in a timely manner, please send a message directly to the development mailing list. If the issue is urgent or sensitive in nature (e.g., a security vulnerability) please send an e-mail directly to the private e-mail [email protected]. |
|
👋 Thank you for your draft pull request! Do you know that you can use |
ef98ccd to
ac63d76
Compare
ac63d76 to
7766b19
Compare
|
oh that's fun: because this only adds an argument to an existing function, it's not rejected by CI even though the corresponding change hasn't landed in a numpy nightly yet. This is because our signature checks are intentionally lenient about wrappers being more flexible that wrapped functions (they just cannot be any stricter). Maybe that could eventually create blindspots, and I'll probably end up opening a dedicated issue about that, but in the mean time, I think I'll just open this for review now, since it can apparently be merged immediately without breaking CI ! |
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
Some suggestions, but really it does not matter, so I will approve now and let you decide whether just to merge what you have, or to follow one of my suggestions.
| ) | ||
|
|
||
|
|
||
| def array_impl(object, *, dtype, copy, order, subok, ndmin, ndmax=0): |
There was a problem hiding this comment.
I'd tend to write the implementation just as
def array_impl(object, **kwargs):
out_unit = getattr(object, "unit", UNIT_FROM_LIKE_ARG)
if out_unit is not UNIT_FROM_LIKE_ARG:
object = _as_quantity(object).value
return (object,), kwargs, out_unit, None
Alternatively, write it as it will be with numpy >=2.4, and then do
if not NUMPY_LT_2_4:
array = function_helper(array_impl)
else:
@function_helper
def array(object, dtype=None, *, copy=True, order="K", subok=False, ndmin=0):
return array_impl(
object, dtype=dtype, copy=copy, order=order, subok=subok, ndmin=ndmin
)
There was a problem hiding this comment.
in any case the difference will factor itself out in the future, so indeed, it shouldn't really matter. Since I'm not near a keyboard right now, I'd rather just merge as is. But I'll try to keep this in mind next time, thanks !
…bility with numpy 2.4
Description
xrefs:
array'sndmaxparameter numpy/numpy#30159array'sndmaxparameter numpy/numpy#30160Again here, I'm ahead on numpy nightlies, so this problem shouldn't be visible on unstable CI before Sunday or Monday.
Hopefully the small issue I reported and patched upstream (see xrefs) is resolved in the mean timeupdate: done