-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Allow plotting categorical data #5464
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
Conversation
Are there any related tests for this that can be extended? |
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.
LGTM! This needs a whats-new note though. Thanks @Illviljan
xarray/plot/plot.py
Outdated
return x[0] - xstep, x[-1] + xstep | ||
|
||
# Try to center the pixels: | ||
left, right = _maybe_center_pixels(x) |
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.
np.issubdtype(v.dtype, str) check hadn't been there.
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.
Has to be refixed if/when ax.imshow allows categorical data.
This reverts commit 3a1c196.
Turns out ax.imshow has some support for strings in the sense that it convert it to integers. So I had to some formatting to get the ticklabels back to strings. a = xr.DataArray(
[[0, 1, 2], [3, 4, 5]],
dims=("dim_0", "dim_1"),
coords=dict(dim_0=(["dim_0"], [0, 1]), dim_1=(["dim_1"], ["u", "v", "w"])),
)
a.plot.imshow() |
Co-authored-by: Deepak Cherian <[email protected]>
Thanks @Illviljan this is great work! |
* main: Improve error message for guess engine (pydata#5455) Refactor dataset groupby tests (pydata#5506) DOC: zarr note on encoding (pydata#5427) Allow plotting categorical data (pydata#5464)
pre-commit run --all-files
whats-new.rst