pint-xarray icon indicating copy to clipboard operation
pint-xarray copied to clipboard

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

Open keewis opened this issue 2 years ago • 3 comments

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?

  • [x] Closes #151
  • [x] Tests added
  • [x] Passes pre-commit run --all-files
  • [x] User visible changes (including notable bug fixes) are documented in whats-new.rst

keewis avatar Jun 06 '22 11:06 keewis

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?

dcherian avatar Jun 06 '22 18:06 dcherian

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 avatar Jun 06 '22 19:06 keewis

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 avatar Jun 21 '22 15:06 keewis