-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Use .to_numpy() for quantified facetgrids #5886
Conversation
xarray/plot/plot.py
Outdated
# Pass the data as a masked ndarray too | ||
zval = darray.to_masked_array(copy=False) | ||
zval = zarray.to_masked_array(copy=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite sure what needs to be done here with regards to converting between quantified arrays and masked arrays
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I think it's already been done in this previous PR, which means .to_masked_array
already dequantifies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... which means that calling .as_numpy()
and then also calling .to_masked_array()
unneccessarily repeats calling .values
, so I got rid of the former.
Did all of these changes print the warning? Would be nice if we could get these tested somehow. |
I don't know - instead of testing all the cases I just made the
I've added some tests for faceted line plots, faceted imshow, and faceted contourf. There is almost certainly some other type of plot or other edge case I haven't tested, but that's enough to show that this PR fixes what it intended to fix. It would be nicer to know for certain that we are testing all the plot types with pint arrays, but I think that can maybe wait for another PR? |
Agreed, it can wait for another time. This looks good to me. :) |
Hello @TomNicholas! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-10-28 22:17:40 UTC |
Thanks @TomNicholas |
* main: Add typing_extensions as a required dependency (pydata#5911) pydata#5740 follow up: supress xr.ufunc warnings in tests (pydata#5914) Avoid accessing slow .data in unstack (pydata#5906) Add wradlib to ecosystem in docs (pydata#5915) Use .to_numpy() for quantified facetgrids (pydata#5886) [test-upstream] fix pd skipna=None (pydata#5899) Add var and std to weighted computations (pydata#5870) Check for path-like objects rather than Path type, use os.fspath (pydata#5879) Handle single `PathLike` objects in `open_mfdataset()` (pydata#5884)
* upstream/main: Add typing_extensions as a required dependency (pydata#5911) pydata#5740 follow up: supress xr.ufunc warnings in tests (pydata#5914) Avoid accessing slow .data in unstack (pydata#5906) Add wradlib to ecosystem in docs (pydata#5915) Use .to_numpy() for quantified facetgrids (pydata#5886) [test-upstream] fix pd skipna=None (pydata#5899) Add var and std to weighted computations (pydata#5870)
Co-authored-by: Illviljan <14371165+Illviljan@users.noreply.github.com>
Follows on from #5561 by replacing
.values
with.to_numpy()
in more places in the plotting code. This allowspint.Quantity
arrays to be plotted without issuing aUnitStrippedWarning
(and will generalise better to other duck arrays later).I noticed the need for this when trying out this example (but trying it without the
.dequantify()
call first).(@Illviljan in theory
.values
should be replaced with.to_numpy()
everywhere in the plotting code by the way)pre-commit run --all-files
whats-new.rst