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 bgyp colormap #32

Merged
merged 1 commit into from
May 2, 2024
Merged

add bgyp colormap #32

merged 1 commit into from
May 2, 2024

Conversation

wx4stg
Copy link
Contributor

@wx4stg wx4stg commented Apr 11, 2024

This is designed to be a correlation coefficient colormap that is more accessible than plasmidis (#24) as it does not contain a lightness reversal and therefore is easier to differentiate high correlation from low correlation areas. It also works better for black and white printing for this reason.

However, it compromises on familiarity (both NWS and plasmidis have a lightness reversal at 0.9 to 0.95 ish). This might be too much of a problem to make it 'not worth it'. I do not have any sort of color vision deficiency, but personally think that bgyp is subjectively ugly (and would vote against it's inclusion if there's a better option, which might already exist in the form of NWS_CC or plasmidis).

So this PR is a bit of grudgingly opened suggestion. Colormaps aren't supposed to be an art competition, and if it helps those with CVD, then it should definitely be included, but I'm marking this as a draft because I want to know that before 'officially' suggesting that this be included.

@wx4stg
Copy link
Contributor Author

wx4stg commented Apr 11, 2024

viscm
colormap-lightness
rdr_plot_newcmap

@zssherman
Copy link
Collaborator

zssherman commented May 2, 2024

@wx4stg I think even if this isn't the best colormap for highlighting etc, but if its still CVD friendly and a user can find use in it, I'm for adding it in. Thanks for the PR!

@zssherman zssherman marked this pull request as ready for review May 2, 2024 15:45
@zssherman zssherman merged commit 0a587aa into openradar:main May 2, 2024
6 checks passed
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