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

Add item to check for pint array type #4751

Closed
wants to merge 3 commits into from

Conversation

max-sixty
Copy link
Collaborator

  • Closes #xxxx
  • Tests added
  • Passes isort . && black . && mypy . && flake8
  • User visible changes (including notable bug fixes) are documented in whats-new.rst

Separate an item that may be causing some failures from #4746

@max-sixty
Copy link
Collaborator Author

This confirms that the issue in #4746 (comment) is this — lots of test failures — https://dev.azure.com/xarray/xarray/_build/results?buildId=4657&view=ms.vss-test-web.build-test-results-tab

Any thoughts on how to fix?

@max-sixty max-sixty changed the title Add item to check for pint array tipe Add item to check for pint array type Jan 3, 2021
@max-sixty max-sixty mentioned this pull request Jan 3, 2021
5 tasks
@keewis keewis closed this Jan 5, 2021
@keewis keewis reopened this Jan 5, 2021
Comment on lines +38 to +44

try:
import pint

pint_array_type = (pint.Quantity,)
except ImportError: # pragma: no cover
pint_array_type = ()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems very strange to me that this would make some of the tests fail. pint didn't release recently, so this should not happen. I tried to rerun, let's see if that changes anything.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to be due to a circular import: pint.compat tries to import xarray objects. pytest.importorskip silences the import error somehow, so we get partially initialized modules. I'm somewhat surprised pint still does that, I thought we switched to comparing names some time ago.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was actually the other way around! We had to change from name-based comparison to actual type checks since both xarray and measurements have a Variable class, and they need to be treated completely differently (see hgrecco/pint#959).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I see. There should be a way to compare without importing (maybe using fully-qualified names like xarray.Variable or xarray.core.variable.Variable), but I guess to really fix this we need figure out #3950.

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.

3 participants