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 colormap endpoints #796

Merged
merged 14 commits into from Mar 21, 2024
Merged

add colormap endpoints #796

merged 14 commits into from Mar 21, 2024

Conversation

vincentsarago
Copy link
Member

@vincentsarago vincentsarago commented Mar 14, 2024

ref #509

Screenshot 2024-03-14 at 4 08 31 PM Screenshot 2024-03-14 at 4 09 20 PM Screenshot 2024-03-14 at 4 09 35 PM Screenshot 2024-03-14 at 4 10 10 PM Screenshot 2024-03-14 at 4 22 20 PM

@vincentsarago
Copy link
Member Author

vincentsarago commented Mar 14, 2024

TODO

  • ask for feedback
  • add tests

@JinIgarashi
Copy link
Contributor

JinIgarashi commented Mar 14, 2024

This new endpoint looks very useful. I would suggest to have a query param for number of classes at the endpoint of /colormap/{id}.
I can see current endpoint returns so many classes (44 No. what I can see in screenshot), but it would be nice if client side can request how many classes to be returned from titiler. I think number of classes parameter can be useful if want to use custom colormap to classify data with limited classes instead of linear legend.

Also, it might be useful if it can return color as hsl format in addition to rgba and hex formats. If color format is following maplibre style spec, it can be easier to adapt for maplibre/mapbox.

@vincentsarago
Copy link
Member Author

@JinIgarashi thank for your feedback 🙏

(44 No. what I can see in screenshot)

in fact it returns 256 values, because all rio-tiler colormaps are composed of values between 0 and 255 (because that's how GDAL colormap are)

We could add a step parameter to only select few values but user would need to understand that for colormaps (e.g qualitative colormaps) this would give really weird results

image

@vincentsarago
Copy link
Member Author

Also, it might be useful if it can return color as hsl format in addition to rgba and hex formats. If color format is following maplibre style spec, it can be easier to adapt for maplibre/mapbox.

Well the more I think about this, the more I think we should only return value as RGBA tuple, not event as HEX string. It seems quite trivial do transform the RGBA values to hex or hsl in client application instead of adding more complexity in the titiler endpoint

@vincentsarago
Copy link
Member Author

This new endpoint looks very useful. I would suggest to have a query param for number of classes at the endpoint of /colormap/{id}.

same as #796 (comment), this would be pretty trivial to do in the client application directly instead of adding complexity in titiler endpoint IMO

@vincentsarago
Copy link
Member Author

Q: we have the tilematrixet endpoints with a /tileMatrixSet prefix (defined by OGC), so should the colormap endpoints be prefixed with colorMaps (instead of colormaps as defined now)?

## This PR 

/colormaps
/colormaps/{colormapId}

## Proposed

/colorMaps
/colorMaps/{colormapId}

@vincentsarago vincentsarago marked this pull request as ready for review March 19, 2024 11:22
@vincentsarago vincentsarago merged commit 9df1731 into main Mar 21, 2024
10 checks passed
@vincentsarago vincentsarago deleted the feature/add-colormap-endpoints branch March 21, 2024 09:15
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