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 turbo colormap #348

Merged
merged 2 commits into from
Nov 4, 2024
Merged

add turbo colormap #348

merged 2 commits into from
Nov 4, 2024

Conversation

jkittner
Copy link
Contributor

@jkittner jkittner commented Oct 31, 2024

This PR adds the popularly used "turbo" color map as a builtin/natively supported colormap. Turbo should be used over "jet". There have been somewhat recent studies highlighting the advantage: https://research.google/blog/turbo-an-improved-rainbow-colormap-for-visualization/ .

I hope I did everything needed - is the static docs file generated automagically when the docs are deployed or any manual work needed?

I generated the files using the generate_cmaps.py script. It yielded a few more additions and a few changes - not sure we want to update/add those? Let me know!

see also:

@dionhaefner
Copy link
Collaborator

Yes, please re-run generate_cmaps.py and commit all changes.

You don't need to pitch the advantages of turbo or any specific color map for that matter, it's just that we're slightly out of sync with matplotlib since that script has to be run manually :)

@jkittner
Copy link
Contributor Author

jkittner commented Nov 4, 2024

Okay, I reran the script and added all changes. I had a look at why the client app build is failing - I feel like that does not work for a pull request coming from a fork? It either can't check out my branch or when not specifying a ref it can't push back the changes - not sure If we need to change that?

@dionhaefner
Copy link
Collaborator

I see, thanks. Should be fine.

Unrelated: I really dislike this enumeration of colormaps in the client. After all, we have the /colormaps to retrieve this information directly from the server. Alas, a problem for another day.

@dionhaefner dionhaefner merged commit 0e776b7 into DHI:main Nov 4, 2024
5 of 8 checks passed
@jkittner jkittner deleted the turbo-cmap branch November 4, 2024 10:29
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.

2 participants