Skip to content
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

Closed
wants to merge 2 commits into from
Closed

Conversation

banesullivan
Copy link
Contributor

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)

import matplotlib
import matplotlib.colors as mcolors

cmap = matplotlib.cm.get_cmap(palette, 255) # 255 might be overkill
Copy link
Contributor Author

@banesullivan banesullivan Dec 7, 2021

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?

Copy link
Member

@manthey manthey Dec 7, 2021

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).

@banesullivan banesullivan changed the title Add support for name MPL colormaps Add support for named MPL colormaps Dec 7, 2021
@manthey
Copy link
Member

manthey commented Dec 7, 2021

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"]).

@banesullivan
Copy link
Contributor Author

banesullivan commented Dec 7, 2021

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 colors module

@banesullivan
Copy link
Contributor Author

This would add palettable as a dependency (optional?) to large_image

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

@manthey
Copy link
Member

manthey commented Dec 7, 2021

Or perhaps MPL actually has this support somewhere?

matplotlib.colors.to_hex does what we'd want: e.g., matplotlib.colors.to_hex('blue') or matplotlib.colors.to_hex('xkcd:blue'). Generally, matplotlib.colors.get_named_colors_mapping().keys() will get a list of all color names it handles.

It would make matplotlib an (optional?) dependency of large-image. palettable doesn't require matplotlib.

@banesullivan
Copy link
Contributor Author

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?

@banesullivan
Copy link
Contributor Author

When deploying a tile serving app, every requirement has a cost and MPL isn't trivial

@manthey
Copy link
Member

manthey commented Dec 7, 2021

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?

There is a package that already does this (mpl-colors).

@manthey
Copy link
Member

manthey commented Dec 7, 2021

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?

There is a package that already does this (mpl-colors).

Nevermind, mpl-colors imports matplotlib.

@manthey
Copy link
Member

manthey commented Dec 13, 2021

I've made PR #724 that supercedes this. It generalizes this behavior for all tile sources and expands its capabilities.

@manthey manthey closed this Dec 13, 2021
@manthey manthey deleted the mpl-colormaps branch February 15, 2022 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants