Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change .pint.magnitude to return a DataArray #172

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Jun 6, 2022

At the moment, this is an alias of .pint.dequantify(), but it could just as well be something similar to:

dequantified = da.pint.dequantify()
dequantified.attrs.pop("units", None)
magnitude = dequantified.pint.quantify()

What do you think, @dcherian, @TomNicholas?

@dcherian
Copy link

dcherian commented Jun 6, 2022

An alias is the simplest.

dequantified.attrs.pop("units", None)

I'm 50/50 on this. I can see it being useful but potentially confusing. Since PintArray.magnitude strips unit info I guess it makes sense to drop it.

magnitude = dequantified.pint.quantify()

What does this do? Is it a unitless non-dimensional array? Is that any more useful than a plain numpy-backed DataArray?

@keewis
Copy link
Collaborator Author

keewis commented Jun 6, 2022

What does this do? Is it a unitless non-dimensional array? Is that any more useful than a plain numpy-backed DataArray?

The idea was that the property would only modify the actual data and not the coordinates. Not sure if that makes sense, though.

@keewis
Copy link
Collaborator Author

keewis commented Jun 21, 2022

Upon further consideration, I'm somewhat tempted to close this PR and point to arr.pint.dequantify(): the alias would be a limited (and thus less useful) version of dequantify, which means that I would probably never use .pint.magnitude.

The main argument we had in the original issue was that to get the wrapped array we'd be able to do arr.data.magnitude while getting the "magnitude" DataArray would be much more difficult. However, I think it's actually the other way around: you can use arr.pint.dequantify() to get a dimensionless DataArray, but as far as I can tell the current behavior of arr.pint.magnitude is closer to getattr(arr.data, "magnitude", arr.data). The current behavior is also what metpy is doing, but we of course don't have to copy that.

That said, as long as we find a consistent definition for DataArray.pint.magnitude I'd be fine with that.

@jthielen, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can .pint.magnitude return a DataArray instead?
2 participants