Repository navigation
NEP35 implementation tracker #17124
Description
This was initially discussed in #17001. This is the larger scope issue.
In short, NEP 35 can be seen as an extension of NEP 18 and provides a different yet similar dispatching mechanism for some array constructor functions in NumPy, which can (and should) be implemented in Quantity and Masked.
Because NumPy itself doesn't explicitly list them1, our completion tests do not report them as missing implementations, and the list needs to be somehow retrieved manually. I'll write it down here as two TODO lists (one for each subpackage) and link relevant PRs
Quantity
- ENH: implement
np.arangedispatch forQuantity#17059 - ENH: add support for NEP 35 array constructors (
empty,ones,zerosandfull) withlike=Quantity(...)) #17120 - ENH: add support for NEP 35 constructors (
array,asarray,asanyarray,ascontiguousarrayandasfortranarray) with like=Quantity(...)) #17125 - ENH: add support for NEP 35 array constructors (
frombuffer,fromfile,fromiter,fromstringandfromfunction) withlike=Quantity(...))#17128 - ENH: add support for NEP 35 array constructors (
require,identiy,eye,tri,genfromtxtandloadtxt) withlike=Quantity(...))#17130
Bonus round
- Once the list is completed, a
What's Newshould be added to our next release notes (done in ENH: add support for NEP 35 array constructors (require,identiy,eye,tri,genfromtxtandloadtxt) withlike=Quantity(...))#17130)
Masked
-
arange -
empty -
ones -
zeros -
full -
array -
asarray -
asanyarray -
ascontiguousarray -
asfortranarray -
require -
fromfunction -
fromstring -
fromiter -
fromfile -
frombuffer -
identity -
loadtxt -
genfromtxt -
eye -
tri
Bonus round
- Once the list is completed, a
What's Newshould be added to our next release notes
FWIW, I'm actively working towards completing the implementation for Quantity in time for astropy 7.0's feature freeze, and would like to tackle Masked too if time allows.
Activity
pllim commentedon Oct 4, 2024
Do we need a
Wow Ambitiouslabel? 😆neutrinoceros commentedon Oct 9, 2024
Now that the
unitspart is done, we can focus the discussion onutils.masked.Reading through NEP 35, I think it would make sense that
np.ones((16, 16), like=Masked(Quantity([], u.cm))would work, but there's an additional layer of indirection that I'm not sure how to overcome. Namely,MaskedQuantitygets its__array_function__implementation fromQuantity, notMasked. I think this warrants adding tests to check that the behavior of NEP 35 functions is not completely bananas for astropy 7.0.0, but maybe not rush the implementation itself in time for the feature freeze (after all, I only proposed this out of love for consistency, but no one actually requested it).So, let me start the conversation with: do we want to support this ?
neutrinoceros commentedon Oct 10, 2024
ping @mhvk just in case
mhvk commentedon Oct 10, 2024
Ah, mixed classes are an interesting bit... For the normal case, inside
Quantity.__array_function__, we do.valuefor every quantity, and this value will beMaskedNDArrayif we're dealing with aMaskedQuantity. This then gets passed on with asuper()call toMaskedNDArray.__array_function__.I think it should work similarly for the NEP 35 functions, at least if they are covered with
function_helper. If they are dispatched, I think one would have to pass on alike=self.value, which would be trickier. So, it may be that my suggestion to usefunction_helperended up being quite lucky...neutrinoceros commentedon Oct 10, 2024
Awesome. Other crucial questions I want to ask upfront:
Does it always make sense to enforce the same rules we defined for
unitspropagation with masks ?1) I'm guessing falling back to the mask from the
likeargument always makes sense (but I'm happy to be shown wrong)2) for functions where other arguments may plausibly be masked arrays too, use their masks for the output instead ?
3) what happens if we get
np.asarray(np.ma.maskedarray([1], [True], like=Masked(np.array([], False))? (I'm asking because I know that numpy.ma masks behave very differently than ours) I'd expect this to still pick the mask from the inputnp.maarray, but convert it toMasked(np.ndarray), but I haven't thought deeply about it. Maybe we should not support such ill defined cases at all (at least for the time being ?) ?mhvk commentedon Oct 10, 2024
My sense would be that it would be a bit different from
Quantity, in that the mask of thelikeargument would never be involved, just like is currently the case forzeros_like,ones_like, etc. This also becauselike's shape would likely not have anything to do with the actual function. But a mask for thefill_valueinfull(..., like=m)would of course be used. So, in your example forasarray, I think one should getMasked([1], [True]).neutrinoceros commentedon Oct 17, 2024
Alright, we have ~8 days before the feature freeze and I have not started implementing the Masked counterpart. It may be still be doable on my end but I can't speak for my reviewer(s). So, before I dive in: @mhvk, assuming I can manage to do all 21 functions by Monday, do you think you'll be able to review by Friday ? If not, that's perfectly okay to me, there are plenty other things to be done, I'm just trying to optimize the time we have left.
mhvk commentedon Oct 17, 2024
Yes, I think I can review that. But I don't think this is particularly urgent and it is not an API change, so only if there aren't more important API affecting changes to do.
neutrinoceros commentedon Oct 18, 2024
Upon reflection I think my time will be better spent trying to help pushing existing PRs accross the finish line. As you said, this is not urgent !
neutrinoceros commentedon Nov 12, 2024
(hopefully) quick question for @mhvk : why is there currently no
@function_helperdecorator in utils.masked ?mhvk commentedon Nov 12, 2024
Unlike
Quantity, there weren't really many cases where one can just drop tondarray, run the function, and stick a mask back on -- from a quick look, there were only a few, likeiscomplexandunwrap, and it looks like I just decided to dispatch them instead. I guess the closest equivalent is@apply_to_both.neutrinoceros commentedon Nov 13, 2024
It looks to me that, if only for the sake of making comparison with the work we did on
Quantityeasier, it would be a good starting point to add such a similar decorator tomaskedfirst. Any objection ?mhvk commentedon Nov 13, 2024
No, no objection, though I'm still not quite sure
@apply_to_bothwouldn't be more appropriate forzeros/ones/empty/full_like(forarange, almost certainly dispatching is best).neutrinoceros commentedon Nov 21, 2024
@mhvk I see that numpy/numpy#27504 was just merged. Should we expect a subtle break in devdeps, or will this allow us to simplify what we did in units later on ?
mhvk commentedon Nov 21, 2024
It just means that we can do
super().__array_function__(...)without fear of breaking. Specifically, it will no longer be necessary to do:astropy/astropy/units/quantity.py
Lines 1900 to 1906 in 157e90a
I guess we can put a
if NUMPY_LT_2_2around thetry/except, and in theelseclause just callsuper()unconditionally (or add a comment in thetry/except- these days, the construct has no cost if no exception is raised).