search

xarray-contrib / pint-xarray

Interface for using pint with xarray, providing convenience accessors
https://pint-xarray.readthedocs.io/en/latest/
Apache License 2.0
101 stars 12 forks source link

change `.pint.magnitude` to return a `DataArray` #172

Closed keewis closed 2 months ago

keewis commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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?

keewis commented 2 months ago

closing, since I think it's actually better to use .pint.dequantify() for this (we might have to allow selectively dequantifying variables, though)