Skip to content

Maybe: Let Quantity.value return an array scalar instead of a numpy scalar #16508

Open

Description

@mhvk

What is the problem this feature will solve?

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.

Furthermore, the repr would change,

>>> q = 5 * u.m
>>> q.value
np.float64(5.0) -> np.array(5.0)

Describe the desired outcome

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(...).

Activity

  1. neutrinoceros commented on May 28, 2024

    @neutrinoceros
    Contributor

    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.

  2. pllim commented on May 28, 2024

    @pllim
    Member

    Is this the problem @larrybradley recently ran into in photutils?

  3. larrybradley commented on May 28, 2024

    @larrybradley
    Member

    @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:

    >>> a = 1 * u.Jy
    >>> np.isscalar(a)
    False
    >>> a.isscalar
    True
    

    I was using np.isscalar because in my real-life example the parameter a could also be a plain float (so .isscalar won't work).

  4. mhvk commented on May 28, 2024

    @mhvk
    ContributorAuthor

    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...

  5. nstarman commented on May 28, 2024

    @nstarman
    Member

    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?

  6. mhvk commented on May 28, 2024

    @mhvk
    ContributorAuthor

    @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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions