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

silence UnitStrippedWarnings #4163

Merged
merged 55 commits into from Jul 2, 2020

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Jun 17, 2020

Now that the tests are cleaned up, it is time to fix the numerous "unit stripped" warnings. We'll get there by promoting all UnitStrippedWarnings to errors (using filterwarnings) and fixing / xfailing the failing tests.

  • Passes isort -rc . && black . && mypy . && flake8
  • Fully documented, including whats-new.rst for all changes and api.rst for new API

@keewis keewis added this to In progress in Duck Array Wrapping via automation Jun 17, 2020
@keewis
Copy link
Collaborator Author

keewis commented Jun 23, 2020

unfortunately, this doesn't seem to really work: VariableSubtests was written to use the same tests for Variable and IndexVariable, and using it to test duckarrays (pint in particular) is difficult since the data that is often used to compare the result is not converted. I'm not sure if rewriting VariableSubtests to optionally allow converting the data or adding specific Variable-with-units tests instead of inheriting from VariableSubtests is easier...

@max-sixty
Copy link
Collaborator

I just saw your message from a couple of days ago @keewis .
To the extent you've solved that, great.
If you haven't — one option would be to change assert_array_equal in those tests to xarray's assert_equal, and ensure xarray's functions handle the duck arrays reasonably.

@keewis
Copy link
Collaborator Author

keewis commented Jun 25, 2020

I tried that by writing some sort of assert_duckarray_equal and assert_duckarray_allclose function. That should work for most duck arrays, but not pint: the tests often compare the data with the variable (or the variable's data), and since the data is mostly a bare numpy array this fails for pint: bare numpy arrays are by definition dimensionless and so the comparison either returns False (the assertion fails) or a DimensionalityError is raised.

So to make this work I think we'd need to rewrite VariableSubtests to have another static method (array_type? data_type?) which is called on all data before it is passed to cls. That, however, has its own set of issues and in the end I concluded that VariableSubtests was just not written with duck arrays in mind, and that pytest doesn't encourage reusing tests by using inheritance (marking or removing tests from base classes is really clunky). With that in mind I simply removed the use of VariableSubtests for now (this shouldn't decrease the coverage too much) and we can add it back if / when we notice that we actually need these tests.

Edit: Right now the only failing tests are due to hgrecco/pint#1123. I guess I'll just remove the xfail condition and add it back once that PR is merged.

@keewis
Copy link
Collaborator Author

keewis commented Jun 26, 2020

Once the CI passes this should be ready for review and merging.

xarray/core/common.py Show resolved Hide resolved
xarray/testing.py Outdated Show resolved Hide resolved
xarray/testing.py Show resolved Hide resolved
xarray/core/common.py Show resolved Hide resolved
@dcherian dcherian mentioned this pull request Jun 29, 2020
23 tasks
@dcherian dcherian merged commit e216720 into pydata:master Jul 2, 2020
Duck Array Wrapping automation moved this from In progress to Done Jul 2, 2020
@dcherian
Copy link
Contributor

dcherian commented Jul 2, 2020

Thanks @keewis. I'm really excited to see this go in.

@keewis keewis deleted the silence-unit-stripped-warnings branch July 2, 2020 17:29
@keewis keewis mentioned this pull request Jul 2, 2020
18 tasks
dcherian added a commit to dcherian/xarray that referenced this pull request Aug 16, 2020
* master:
  Add initial cupy tests (pydata#4214)
  Add 0.16.0 release summary
  New whatsnew section
  Release v0.16.0
  Minor reorg of whatsnew for 0.16.0 (pydata#4216)
  fix sphinx warnings (pydata#4199)
  pin isort (pydata#4206)
  get the colorbar label via public methods (pydata#4201)
  Bump minimum versions for 0.16 release (pydata#4175)
  Allow passing axis kwargs to plot (pydata#4020)
  Fix to_unstacked_dataset for single dimension variables. (pydata#4094)
  Improve the speed of from_dataframe with a MultiIndex (by 40x!) (pydata#4184)
  More pint compatibility: silence UnitStrippedWarnings (pydata#4163)
  Fix typo (pydata#4192)
  use the latest image of RTD (pydata#4191)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants