-
Notifications
You must be signed in to change notification settings - Fork 12
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
avoid setting force_ndarray_like = True
#216
Comments
If we can make sure the
I'm not saying that I insist on setting that option, it's just that I don't think there is a way to keep |
By modifying global state at import time, the behaviour of my program depends on if
In our specific case, it breaks code that serialises python quantity scalars to json, which has a custom json converter for Would it be possible to at least delay automatic patching of the registry to the first actual use of anything |
As I said, I'm not sure what to do about this. However, I really would like to avoid doing this on first use of the import pint_xarray
q = pint.Quantity(0, "m")
ds1 = xr.Dataset({"a": ("x", q)}) # UnitStrippedWarning
ds2 = xr.Dataset({"a": ("y", 0, {"units": "m"})}).pint.quantify() # works, since it patches the registry
ds3 = xr.Dataset({"a": ("x", q)}) # works whereas clearly stating that the import patches the registry makes the behavior a lot easier to understand, both as a user and when reading / modifying the code (even though I agree that modifying global state is not good design and should be avoided as much as possible). Considering that, I think our time is best spent figuring out how we can change In this particular case, though, I wonder if you have considered extending the custom json converter to treat 0d numpy arrays the same as scalars? That would allow you to side-step the issue entirely. For example, you could try normalizing the object with: normalized = getattr(obj, "tolist", lambda: obj)() or if hasattr(obj, "tolist"):
normalized = obj.tolist()
else:
normalized = obj |
Of course, the fix in our code wouldn't be particularly difficult. One can argue that any generic code dealing with a pint quantity today will have to be prepared for array scalars - if it isn't, it's broken.
You are probably right. |
I'm again very much tempted discard In this particular case, I have a It may not be possible to fix that from within |
It's just that: Requiring a change to a global setting make What was the reasoning again that led to the conclusion that I'll submit a pull-request shortly, so we may get some feedback from CI as-well. |
The workflows will require some approval before we get CI feedback... |
the relevant issues are hgrecco/pint#950 and hgrecco/pint#1342. You won't see this in As a summary, though, the issue is this: In [1]: import pint
...: import xarray as xr
...:
...: ureg1 = pint.UnitRegistry()
...: ureg2 = pint.UnitRegistry(force_ndarray_like=True)
In [2]: import pint
...: import xarray as xr
...:
...: ureg1 = pint.UnitRegistry(force_ndarray_like=True)
...: ureg2 = pint.UnitRegistry()
In [3]: xr.Dataset({"a": ((), ureg1.Quantity(1, "m"))})
Out[3]:
<xarray.Dataset>
Dimensions: ()
Data variables:
a int64 <Quantity(1, 'meter')>
In [4]: xr.Dataset({"a": ((), ureg2.Quantity(1, "m"))})
...
AttributeError: Neither Quantity object nor its magnitude (1) has attribute 'shape' While I'm sure we could define |
So arguably the root problem here is that
Is that the case? Or is the way in which pint is divergent not actually covered by the (current) array API standard? |
|
Thank you @keewis for all the details.
Array API support in pint has been suggested here hgrecco/pint#1592, but I don't see any evidence of progress yet. That should really be the long-term aim though - pint should support the array API, so pint-xarray can stop setting |
It's also really on the pint devs, not on the xarray team. (I'm also very happy to see that you're working on #163, and happy to help out with finishing #143 after that!) In the meantime what's the best way forward here? Does this problem only come up at the point we call |
I believe it may be more complicated than that.
Thus, I don't think there is all too much that The fundamental problem indeed remains Given that so many things break when scalars do not behave like scalars, I wonder if there is really no way to introduce a workaround around |
Moving to a separate issue to be able to properly track this.
Originally posted by @burnpanck in #189 (comment)
Originally posted by @keewis in #189 (comment)
Originally posted by @burnpanck in #189 (comment)
The text was updated successfully, but these errors were encountered: