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

importing pint_xarray breaks Quantity.__round__ #189

Closed zobac closed 1 year ago

zobac commented 1 year ago

When pint_xarray is imported it sets pint.application_registry.force_ndarray_like = True Once this is done rounding Quantities raises an exception because ndarrays have no round method.

import pint
UREG = pint.UnitRegistry()
pint.set_application_registry(UREG)
round(UREG.Quantity('1.378533532 m'))
<Quantity(1.0, 'meter')>
import pint_xarray
round(UREG.Quantity('1.378533532 m'))
Traceback (most recent call last):
  Python Shell, prompt 6, line 1
    # Used internally for debug sandbox under external interpreter
  File "E:\Envs\xarray311\Lib\site-packages\pint\facets\plain\quantity.py", line 1395, in __round__
    return self.__class__(round(self._magnitude, ndigits=ndigits), self._units)
builtins.TypeError: type numpy.ndarray doesn't define __round__ method
keewis commented 1 year ago

indeed, this is unfortunate, but I don't think we can avoid setting that option: pint_xarray needs that in a few other places.

Note that you can use np.round instead, with the caveat that np.round will not promote to int if decimals=0.

zobac commented 1 year ago

I see. I'm not actually calling round anywhere. It's called by unittest.TestCase.assertAlmostEqual, which is used liberally in my test suite.

I'm extending pint, defining my own Quantity sub-class. If I define my own magnitude property, to always return a float, will that break using my Quantity class in pint_xarray?

keewis commented 1 year ago

I guess then the answer might be to use pint_xarray.testing.assert_allclose or pint.testing.assert_allclose instead?

will that break using my Quantity class in pint_xarray?

I never tried that, so I'm not sure? The best way to answer that would be to modify xarray's tests (xarray/tests/test_units.py) to use your custom Quantity, and see what fails.

zobac commented 1 year ago

I guess then the answer might be to use pint_xarray.testing.assert_allclose or pint.testing.assert_allclose instead?

Yep, that works. Thanks. I guess you can close this issue. Not sure if that's something you do or I do...

keewis commented 1 year ago

I think both? I can always reopen if I think it's still an issue...

burnpanck commented 1 year ago

Why does pint-xarray require pint.application_registry.force_ndarray_like = True in the first place? I don't like when my dependencies force me to force something... This has become an issue because that very same setting is incompatible with pint-pandas: https://github.com/hgrecco/pint-pandas/issues/165. Given that xarray bases on pandas, this seems to be somewhat serious.

keewis commented 1 year ago

The reason is that xarray detects a duck array using a certain interface, casting everything that does not satisfy that to numpy using np.asarray. Since python scalars (float, int) don't have the ndim, dtype and shape properties (though pint does default ndim to 0), the result of some numpy functions will be cast, dropping the units.

Requiring force_ndarray_like avoids that, since pint will do the casting before xarray can do it. I agree that this feels like a hack, though.

burnpanck commented 1 year ago

Hm. I have a feeling that this is again a reason for implementing separate pint.QuantityArray and pint.QuantityScalar types - because IMHO pint should never respond to np.asarray by stripping units.

Either way, pint-pandas does not require it, and managed to get most operations to work without stripping units. There, it is up to the user to make sure that their objects make it into pandas as a PintArray, e.g. by setting force_ndarray_like = True, or simply by directly building a PintArray. But once the quantity arrays are in, pandas operations do not strip units (at least things improved significantly). Given that, in most cases, users of both xarray and pandas work with arrays anyways, it feels like forcing force_ndarray_like = True on every user is rather harsh. Even more so by doing it at import time - causing an import order dependency!