-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add support for named MPL colormaps #714
Conversation
import matplotlib | ||
import matplotlib.colors as mcolors | ||
|
||
cmap = matplotlib.cm.get_cmap(palette, 255) # 255 might be overkill |
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.
255 is probably major overkill. @manthey, any idea what might be a more reasonable value here?
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.
You don't need to specify the length. That is,
cmap = matplotlib.cm.get_cmap(palette)
will yield the same as cmap = matplotlib.cm.get_cmap(palette, len(maplotlib.cm.get_cmap(palette).colors))
. And cmap.N == len(cmap.colors)
.
I think the colormaps (both from palettable and what you are adding here) should ideally be moved up to the base tilesource class, rather than exist just for the GDAL source (and mapnik, since it inherits from GDAL). This would add palettable as a dependency (optional?) to large_image. I'll make an issue for this, as it is beyond what this PR should be trying to do. It would also be nice if you could specify a palette as a single color (e.g., "palette": "green", which would expand to "palette": ["black", "green"]). |
This would be pretty easy to implement as well. We could borrow some code from PyVista to have big list of colors and their hex values: https://github.com/pyvista/pyvista/blob/main/pyvista/plotting/colors.py Or perhaps we just use MPL for this - but if MPL is a soft dependency, then user won't be able to use these colors - which is exactly why PyVista has that |
Palettable is pure Python with no dependencies so I think it's absolutely okay to make a hard dependency of large_image. MPL on the other hand should remain optional |
It would make matplotlib an (optional?) dependency of large-image. palettable doesn't require matplotlib. |
I think it's worth having general, named colors support without requiring MPL - to do that, should we make a big list of colors, effectively taking the code out of MPL and putting it in large_image? |
When deploying a tile serving app, every requirement has a cost and MPL isn't trivial |
There is a package that already does this (mpl-colors). |
Nevermind, mpl-colors imports matplotlib. |
I've made PR #724 that supercedes this. It generalizes this behavior for all tile sources and expands its capabilities. |
These changes are adapted from what I implemented here in
localtileserver
to support using MPL colormaps by name:https://github.com/banesullivan/localtileserver/blob/0c444034f63165857c57f4043a05cb73b5df5b71/localtileserver/palettes.py#L56-L63
This should make it so that any Matplotlib named colormap can be used as a
'palette'
when MPL is installed. https://matplotlib.org/stable/tutorials/colors/colormaps.html(I haven't tested this locally yet)