Skip to content

994 exploration colormap #1107

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

Closed
wants to merge 21 commits into from
Closed

994 exploration colormap #1107

wants to merge 21 commits into from

Conversation

dzole0311
Copy link
Collaborator

Related Ticket: {link related ticket here}

Description of Changes

{Update with description of changes made in this pull request}

Notes & Questions About Changes

{Add additonal notes and outstanding questions here related to changes in this pull request}

Validation / Testing

{Update with info on what can be manually validated in the Deploy Preview link for example "Validate style updates to selection modal do NOT affect cards"}

@dzole0311 dzole0311 marked this pull request as draft August 16, 2024 10:10
Copy link

netlify bot commented Aug 16, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit 9192303
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/66bf41b317047800085be880
😎 Deploy Preview https://deploy-preview-1107--veda-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@j08lue
Copy link
Contributor

j08lue commented Aug 16, 2024

I had a first look - awesome to see this feature materialize!

🎉 switching between colormaps and reversing them works - the tiles on the map change and use the right colormaps

🐞 - the legend graphics is off - the tiles are shaded the way I would expect (knowing Viridis or Plasma), but the graphics in the drop-down do not match.

image

@dzole0311
Copy link
Collaborator Author

the legend graphics is off

Yeah, I think this is because I tried to rescale the colors based on the rescale values. I've removed this now since it makes the legend unclear.

@j08lue
Copy link
Contributor

j08lue commented Aug 16, 2024

Ah, I see. I think we will always want to make use of the full color range. We only limit the data range.

So there should not be a situation where the colormap would be rescaled / cropped, as far as I can see.

display: none;
`;

export const SEQUENTIAL_COLORMAPS = [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Should we have a step to validate these colormaps are supported color maps?( from https://openveda.cloud/api/raster/colorMaps)? In reality, the supported colormap will change so rarely that I am not sure if we need to add this one more step, but then we will be safe from whichever update from the backend.

Copy link
Contributor

@j08lue j08lue Aug 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I was hoping that we were doing that - fetching the supported colormaps from TiTiler instead of relying on another source (D3).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see; I missed that there is even an endpoint for getting the colors of each colormap using each colormap ID - ex. https://openveda.cloud/api/raster/colorMaps/magma. Then it might make sense to have only IDs for colormaps on the client side and get the values from the endpoint.

Copy link
Collaborator Author

@dzole0311 dzole0311 Aug 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware of the endpoint as well, thanks @j08lue!

@hanbyul-here Since we use d3-scale-chromatic already which supports (I think most of) the colorMaps by ID and to avoid sending requests on each selection of a colorMap to fetch the values, I don't think we need to hardcode them at all in the UI. In general, we could fetch the list from titiler, list them in the UI as options to choose from and use d3-scale-chromatic to interpolate the values. What do you think about this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, don't we want to curate the colormaps (not all the colormap options from the colormap endpoint)? I meant the IDs for these curated colormaps can be on the front end. (Sorry for not being clear.)

I think it is always best to have one source of truth, so we don't have to think about how the colors from Titiler don't seem to match 100% with what the legend shows. (If that ever happens - So we probably should at least check where D3 colormap values are from and how the values match up with the titiler values.) I also understand your concerns about not making too many requests. Since this is the type of data that doesn't change very often, I wonder if we can make a request while building and save it as a local file? 🤔 (This sounds like a way to make this ticket go out of the scope 😅 )

Copy link
Collaborator Author

@dzole0311 dzole0311 Aug 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So titiler uses colorMaps from rio-tiler. And rio-tiler uses colorMaps (at least some that are currently on the client-side) derived from Matplotlib: https://cogeotiff.github.io/rio-tiler/colormap/#colormaps

d3-scale-chromatic also derives the ones we use from Matplotlib, which is why I think we will add unnecessary overhead if we send requests to fetch hexes per colorMap ID or save them in local files.

Oh, don't we want to curate the colormaps (not all the colormap options from the colormap endpoint

Yes I think this was the idea, to curate a selection of most widely used colors instead of showing all the supported ones. Users could still pass in a valid colorMap name which is not in the list at the moment (e.g. PuRd) and it should show up fine as a default value.

EDIT:

d3-scale-chromatic also derives the ones we use from Matplotlib

Ah, I have no proof for all of the colormaps though, only some. So it might make sense to save the ones which we're unsure of as hex objects in the veda-ui and simply use them. On the other hand, the colors seem to match what the tiles endpoint returns 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the pros and cons of all options. I would really like a solution that makes maximum use of the nice TiTiler built-ins. But the decision is up to you!

I lined up the options here, for overview (using an ADR template):

Please feel free to review and let me know if I should open a proper ADR PR.

@dzole0311 dzole0311 closed this Aug 21, 2024
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.

3 participants