-
-
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
Allow plotting categorical data #5464
Merged
dcherian
merged 18 commits into
pydata:master
from
Illviljan:Illviljan-plot_categorical_data
Jun 21, 2021
Merged
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
f7e5084
Allow plotting categorical data
Illviljan 0a6cce9
change tests
Illviljan 2ccfa6b
Update plot.py
Illviljan 21c028f
Update plot.py
Illviljan a393e91
Update plot.py
Illviljan eb96ee9
ismhow and plot_surface doesn't support cats
Illviljan f6ff2d2
Update plot.py
Illviljan bd9f9ba
Update plot.py
Illviljan c346c25
Better comments
Illviljan f922ce3
Update whats-new.rst
Illviljan 3a1c196
Don't handle pixel centering when inputs are strings.
Illviljan c22b3be
Revert "Don't handle pixel centering when inputs are strings."
Illviljan a9e3826
Assume matplotlib handles the categoricals.
Illviljan e1c45fd
Update plot.py
Illviljan 3e6799b
imshow has some support for strings.
Illviljan 1dc00c1
imshow test doesn't need to raise anymore
Illviljan f08f22d
catch plot_surface error
Illviljan 43d6696
Update xarray/tests/test_plot.py
Illviljan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
doesn't this contradict the error checking above?
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.
This makes sure that xarray isn't the one throwing errors before getting to ax.imshow, if the
np.issubdtype(v.dtype, str)
check hadn't been there.But it's true it's not really necessary as ax.imshow doesn't seem to support categoricals.
But maybe we prefer matplotlib raising the error?
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.
but it is there :) It's redundant, is it not? Shall we just remove the str check from
_maybe_center_pixels
?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.
The other alternative is to remove the other check and let it crash within matplotlib. But I'm not sure they even intend to support categoricals for these functions.
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.
I think i like that. If matplotlib starts supporting things, then it will just work automatically in xarray. I also liked your refactoring to
_maybe_center_pixels
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.
Ok, then I just need to figure out how to catch the errors in the tests.