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

do not force_ndarray_like #234

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

Conversation

burnpanck
Copy link

@TomNicholas
Copy link
Collaborator

TomNicholas commented Dec 14, 2023

Thanks for opening this @burnpanck .

I notice in tests/test_accessors.py we also separately set force_ndarray_like=True

unit_registry = UnitRegistry(force_ndarray=True)

That seems odd in that it doesn't fully decouple the tests from the code, and also seems like it might be making this PR misleading by causing tests to pass in the CI that would not work in user code?

@burnpanck
Copy link
Author

Didn't spot that one. However, the tests still pass on my machine. To me, this suggests that force_ndarray_like is really not needed for any of the core xarray/pint interop to work. The only case that has been offered is the automatic coercion of a scalar quantity into a zero-dimensional array in the construction of a DataArray. But I strongly suggest that we add a test for that if we even want to claim that this is an actual feature.

@keewis
Copy link
Collaborator

keewis commented Dec 15, 2023

As I pointed out in #216 (comment), it is possible to use pint + xarray without pint-xarray, so most of the tests that do check compatibility live in xarray's test suite (for now, there's the desire to move those tests here once we have a replacement that still does extensive checks of the duck array compatibility).

After changing the unit registry of these tests to have force_ndarray_like=False 107 tests are failing, so clearly this is important for pint support in xarray.

@burnpanck
Copy link
Author

it is possible to use pint + xarray without pint-xarray

I'll have to process that. I'm wondering why one should be using pint-xarray in the first place then. I'll try to have a go at reproducing those failing tests in xarray so I understand better.

@keewis
Copy link
Collaborator

keewis commented Dec 15, 2023

pint-xarray is a glue / convenience package: while it is possible to use (most of) pint + xarray without pint-xarray, it is not very convenient to do so: creating quantities, converting between them, and serializing back to array + attribute would be very complex otherwise (however, this will change with #163, which will allow having units on indexed coordinates, something that currently is not possible).

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.

avoid setting force_ndarray_like = True
3 participants