You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Maybe: Let Quantity.value return an array scalar instead of a numpy scalar #16508
Currently, for a scalar Quantity, .value returns a numpy scalar. It would make the implementation easier (and more performant) if we just return a zero-dimensional array (i.e., just do .view(np.ndarray).
A side effect would be that the scalar then is a view of the quantity (just like is the case for non-scalar), and hence writeable.
Simplified code, and consistent behaviour between array and scalar quantities.
Though at a cost of an API change, and with the knowledge that elsewhere (e.g., Time), we also special-case scalars in, e.g., allowing them to be pickled.
Additional context
Leftover question from the transition of our docs to numpy 2.0, and hence a lot of cases where now a result is shown as np.float64(...).
I think this is worth doing if only for the sake of API consistency. It's a nice side effect of changes from numpy 2.0 that it forces us to reflect on this stuff.
A bit off-topic, but the documentation for np.isscalar notes "In most cases np.ndim(x) == 0 should be used instead of this function, as that will also return true for 0d arrays." Of course, those types of tests are quite slow...
@nstarman - indeed, if Quantity becomes a proper wrapper, the logical thing becomes for .value just to return whatever it wraps. Since that is hopefully something I'll be working on, it may make most sense not to make smaller API changes beforehand.
Activity
neutrinoceros commentedon May 28, 2024
I think this is worth doing if only for the sake of API consistency. It's a nice side effect of changes from numpy 2.0 that it forces us to reflect on this stuff.
pllim commentedon May 28, 2024
Is this the problem @larrybradley recently ran into in photutils?
larrybradley commentedon May 28, 2024
@pllim I never reported that issue that was discussed on Slack. I'm not sure this would fix it.
Here's an example of my issue, which was quite surprising:
I was using
np.isscalarbecause in my real-life example the parameteracould also be a plain float (so.isscalarwon't work).mhvk commentedon May 28, 2024
A bit off-topic, but the documentation for
np.isscalarnotes "In most casesnp.ndim(x) == 0should be used instead of this function, as that will also return true for 0d arrays." Of course, those types of tests are quite slow...nstarman commentedon May 28, 2024
This is also solved by https://github.com/nstarman/astropy-APEs/blob/units-quantity-2.0/APE25/report.pdf, wherein the new Quantity class is free to hold more data types. So an input scalar remains a scalar, and a PyTorch array remains as such as well.
I think @mhvk is going to be working on this, so perhaps it's easiest to wait?
mhvk commentedon May 28, 2024
@nstarman - indeed, if
Quantitybecomes a proper wrapper, the logical thing becomes for.valuejust to return whatever it wraps. Since that is hopefully something I'll be working on, it may make most sense not to make smaller API changes beforehand.