Fix contourf set under #3601
Fix contourf set under #3601
Conversation
Copies the cmap._rgba_bad, cmap._rgba_under, and cmap._rgba_over values to new_cmap, in case they have been set to non-default values. Allows the user to customize plots more by using matplotlib methods on a cmap before passing as an argument to xarray's plotting methods. Previously these settings were overridden by defaults when creating the cmap actually used to make the plot.
If the input cmap is a str, getting _rgba_bad, _rgba_under, or _rgba_over attributes from it will fail. To fix this, provide defaults taken from new_cmap.
There was a problem hiding this comment.
Apologies for the late review @johnomotani. Thanks for contributing!
Make one unit test test all three properties, rather than having a separate test for each.
|
Not sure why some tests are failing... The ones I can see are to do with |
|
that's #3673, you can ignore these |
When modifying a colormap, we only want to preserve the under and over colors of the original colormap if they were explicitly set by the user. In _build_discrete_cmap this makes no difference, as the new_cmap returned by mpl.colors.from_levels_and_colors has the same minimum and maximum colors as its input, so the default under and over colors would not change anyway and could be copied regardless. However, for clarity and in case the same pattern is needed in future elsewhere, it is nicer to add a checks for: whether the under color is the same as cmap(0), only setting under for new_cmap if it is not; and whether the over color is the same as cmap(cmap.N - 1), only setting over for new_cmap if it is not.
|
I think (some of) the test failures are unrelated ( |
johnomotani
force-pushed
the
fix-contourf-set_under
branch
from
966fa4c to
55fa277
Compare
cmaps contain tuples member variables, so safer to deepcopy instead of just copy to make sure we never change the copied variable.
There was a problem hiding this comment.
Thanks @johnomotani . I've left some minor comments. This is looking good otherwise.
Set vmin and vmax so that _build_discrete_colormap is called with extend='both'. extend is passed to mpl.colors.from_levels_and_colors(), which returns a result with sensible under and over values if extend='both', but not if extend='neither' (but if extend='neither' the under and over values would not be used because the data would all be within the plotted range)
johnomotani
force-pushed
the
fix-contourf-set_under
branch
from
5bb0247 to
061ddf2
Compare
|
I've added an extra test that checks the right thing happens when we do not change the bad, under or over colors. That test needs to set I think this is ready now. The unit test failures on my local computer appear to be unrelated to this PR (see earlier references to #3673 by @keewis and #3747 by @mathause). |
…under * upstream/master: (71 commits) Optimize isel for lazy array equality checking (pydata#3588) pin msgpack (pydata#3793) concat now handles non-dim coordinates only present in one dataset (pydata#3769) Add new h5netcdf backend phony_dims kwarg (pydata#3753) always use dask_array_type for isinstance calls (pydata#3787) allow formatting the diff of ndarray attributes (pydata#3728) Pint support for variables (pydata#3706) Format issue template comment as md comment (pydata#3790) Avoid running test_open_mfdataset_list_attr without dask (pydata#3780) remove seaborn.apionly compatibility (pydata#3749) Python 3.8 CI (pydata#3727) PKG: Explicitly add setuptools dependency (pydata#3628) update whats-new Typo in Universal Functions section (pydata#3663) Release v0.15.0 fix setup.cfg Documentation fixes (pydata#3732) Remove extra && in PR template (pydata#3730) Remove garbage text inserted in DASK_LICENSE (pydata#3729) Avoid unsafe use of pip (pydata#3726) ...
|
Thanks @johnomotani |
Copies the
cmap._rgba_bad,cmap._rgba_under, andcmap._rgba_overvalues tonew_cmap, in case they have been set to non-default values. Allows the user to customize plots more by using matplotlib methods on acmapbefore passing as an argument to xarray's plotting methods. Previously these settings were overridden by defaults when creating thecmapactually used to make the plot.I'm not a fan of copying attributes one-by-one like this, but I guess this is an issue with matplotlib's API, unless there's a nicer way to convert a
cmapto a discretecmapthanmpl.colors.from_levels_and_colors().black . && mypy . && flake8