| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I like this addition and would find it very useful. I wonder if get_cmap should also return a copy by default. I can imagine that would sufficiently API-breaking that we need to think hard about it. But a get_cmap function that accepts these params as well would be quite nice. |
Sorry, something went wrong.
|
Alternatively one could argue that get_cmap() gets you an existing colormap, and e.g. something like new_colormap() gets you a copy of an existing one. This would allow the notably useful API to be incorporated without backwards compatibility issues, plt.cm.new_colormap("viridis", bad="white")
|
Sorry, something went wrong.
|
Initially I thought that changing get_cmap to return a copy would be too disruptive, but let's look at the various behavriors of get_cmap:
Thus, I think modifying get_cmap to return a copy is more likely to fix accidental modifications of builtin or custom cmaps rather than be disruptive. On the other hand, I would say that get_cmap(...).with_extremes(bad=...) (or new_cmap(...).with_extremes(...) -- throughout this paragraph, read whatever spelling you prefer) is not that worse than get_cmap(..., bad=...), and that with_extremes has the advantage of also working with non-builtin cmaps (and having the functionality both in get_cmap and in with_extremes means having to duplicate the code and the docs, yada yada). Finally, having the parameters in get_cmap also modify a non-builtin-colormap passed as argument would be inconsistent with the N parameter of get_cmap, which, for the better or (mostly?) the worst, does not resample non-builtin cmaps. |
Sorry, something went wrong.
|
As you've pointed out get_cmap has quite some odd behavior. I think it should just return a registered colormap and that should be immutable. No passing through of Colormap inputs, no resampling etc. everything else should be done by creating new colormaps from existing colormap objects. Since Colormaps are currently not immutable, get_cmap should return copies of the internal colormaps. It then makes sense to have Colormap.resampled() and Colormap.with_extremes(). Maybe even better to have just one function Colormap.copy(N=None, bad=None, under=None, over=None) - naming is open for better suggestions. Side remark: I'm also not sure if the segmentation N should be a is a fundamental aspect of all colormaps. E.g. one could imagine colormapping by functions. OTOH maybe there is just no need for this and you can always approximate the function on N segments. |
Sorry, something went wrong.
See changelog. See also the number of explicit copies in the examples, which suggest that the old setter-based API is a bit of a footgun (as forgetting to copy seems easy).
|
I really like this pattern. |
Sorry, something went wrong.
|
This does seem useful. Wasn't sure how it fit in w/ @timhoffm plans for the colormap registry... |
Sorry, something went wrong.
|
This does not interfere with my plans for the colormap registry (#18503). The registry only conerns registered colormaps. You can still have separate colormap objects and create new ones. In fact, with_extremes() would still be the canonical way to use custom extremes on a registered colormap, e.g. (API not finalized): my_cmap = mpl.colormaps['jet'].with_extremes(...) |
Sorry, something went wrong.
| @@ -0,0 +1,13 @@ | |||
| `.Colormap.set_extremes` and `.Colormap.with_extremes` | |||
There was a problem hiding this comment.
I think we've decided not to put links in section titles for aestethic reasons.
But I'm going to merge as is. I'll anyway do a small colormap-related cleanup PR and will update there. While that's a bit of a messy way of working it saves us an extra review roundtrip here.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
See changelog. See also the number of explicit copies in the examples,
which suggest that the old setter-based API is a bit of a footgun (as
forgetting to copy seems easy).
PR Summary
PR Checklist