-
Notifications
You must be signed in to change notification settings - Fork 224
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
Use colorblind-friendly colors in the scatter plots gallery example #1013
Conversation
💖 Thanks for opening this pull request! 💖 Please make sure you read our contributing guidelines and abide by our code of conduct. A few things to keep in mind:
|
examples/gallery/plot/scatter.py
Outdated
@@ -21,7 +21,7 @@ | |||
projection="X10c/10c", | |||
frame=["xa0.2fg", "ya0.2fg", "WSrt"], | |||
) | |||
for color in ["blue", "orange", "green"]: | |||
for color in ["seagreen2", "chocolate1", "mediumpurple1"]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for color in ["seagreen2", "chocolate1", "mediumpurple1"]: | |
for color in ["seagreen2", "chocolate1", "mediumpurple1"]: |
Not sure if these colors solve the problem (please correct me if I'm wrong). Maybe it would be better to select colors from cpts that really focus on the color blindness issue.
Here's a deuteranopia (green-blind) simulation of your example @vitorgt:
By the way, here's a nice article about the use (or misuse) of color in scientific visualization by Crameri et al.: https://www.nature.com/articles/s41467-020-19160-7.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but colorblind friendly colorschemes are very limited, and transparency makes this case even worst, I've changed green to a grey, this change will produce greater differences on protanopia and deuteranopia, but purple will get closer to grey on tritanopia (the rarest of the three).
Everywhere i looked on how to choose colorblind friendly colors they say not to rely only color, but to create other ways to differentiate set, usualy changing marker. So i added this.
Also, i couldn't find colorblind friendly cpts on gmt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transparency makes this case even worst,
Perhaps we can try to increase the transparency slightly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transparency makes this case even worst,
Perhaps we can try to increase the transparency slightly?
Agree, perhaps transparency=50
gives a better result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, @michaelgrund, "transparency makes this case even worst" means we sould remove transparency.
Like transparency=90
or remove that line completely (which would be equal to transparency=100
(default)).
Transparenncy makes it worst beacuse it blends the colors and changes their luminance when adding two or more symbols with equal or different colors, notting that color addition with colorblindness is a little bit tricky. That's why transparency should be avoided if we want it more coverage considering colorblindness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're getting a bit confused here. transparency=0
means opaque while transparency=100
means invincible. We probably want to reduce transparency (i.e. increase opacity).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right @weiji14. I think we should not drop transparency here since it allows to deal with overplotting.
Colors chosen based on (https://colorbrewer2.org/#type=qualitative&scheme=Dark2&n=3)
Converted the PR to draft to save resources. |
examples/gallery/plot/scatter.py
Outdated
# and set the circle size to be 0.25 cm (+S0.25c) in legend | ||
label=f"{color}+S0.25c", | ||
# and set the style size to be 0.25 cm (+S0.25{style}) in legend | ||
label=f"{color}+S0.25{style}", | ||
transparency=70, # set transparency level for all symbols |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transparency=70, # set transparency level for all symbols | |
transparency=50, # set transparency level for all symbols |
Perhaps 50 gives a better result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please try 50, i.e. less transparency (more opaque). See also #1013 (comment).
examples/gallery/plot/scatter.py
Outdated
# and set the circle size to be 0.25 cm (+S0.25c) in legend | ||
label=f"{color}+S0.25c", | ||
# and set the style size to be 0.25 cm (+S0.25{style}) in legend | ||
label=f"{color}+S0.25{style}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the warning at https://pygmt-ln1ssu8ap-gmt.vercel.app/gallery/plot/scatter.html#sphx-glr-gallery-plot-scatter-py:
plot [WARNING]: Length <unit> s not supported - revert to default unit [cm]
plot [WARNING]: Length <unit> t not supported - revert to default unit [cm]
0.25c
means 0.25 cm, so we shouldn't use style
as the unit. If we'd like to change the size for each symbol in the legend, it'd better to also add it in the for loop. But I feel 0.25c
is okay for those three symbols.
label=f"{color}+S0.25{style}", | |
label=f"{color}+S0.25c", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much, and I'm sorry, I didn't know that, I thought it was the symbol to show, not the size in cm. Appreciated.
examples/gallery/plot/scatter.py
Outdated
sizes=sizes, | ||
color=color, | ||
# Set the legend label, | ||
# and set the circle size to be 0.25 cm (+S0.25c) in legend | ||
label=f"{color}+S0.25c", | ||
# and set the style size to be 0.25 cm (+S0.25{style}) in legend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# and set the style size to be 0.25 cm (+S0.25{style}) in legend | |
# and set the style size to be 0.25 cm (+S0.25c) in legend |
I agree with it, but I think the example is focusing on different colors, not "different colors and different symbols" (we could add another example for it). I think we should only make changes to colors in this PR. |
@seisman, I kinda disagree with that, the example says:
It doesn't specify its focus, nor restrics the "legend" or the "label" to deal only with colors, and I reaffirm, color blindness accessibility must not rely only on colors, but I'm not a member of GMT, you have far more context within the project than I do, I'll do whatever you members think fits. Before I continue I need to know:
|
Apologies for the late reply @vitorgt, the colours look great! But we could decrease the transparency a bit (i.e. increase opacity) as suggested at #1013 (comment)
Could you try changing it back to circles only to see what it looks like? Oh, and just FYI, we have a Continuous Documentation setup, so you can check the live preview at https://pygmt-git-fork-vitorgt-patch-1-gmt.vercel.app/gallery/plot/scatter.html. Sorry for not mentioning it earlier. |
Personally, I find that the transparency makes it more difficult to see the data points and would prefer to see the transparency parameter removed as @vitorgt suggested. Why not just reduce the overplotting? Gaussian noise alone isn't that interesting to plot anyways. Something like this could be a good compromise (warning: the plotting parameters are not pretty and the comments aren't accurate - just showing the idea here): Edit: one more point: I think adding
|
Thanks, @vitorgt for addressing this issue. Since the transparency/overplotting was not part of the initial accessible colors issue, it can be addressed separately and to wrap this up you can commit the suggestions from @core-man and @seisman and the reduced transparency suggestion by @michaelgrund whenever you get a chance. Here are specific answers to your previous questions:
No, your points are well made and the new symbol selections are fine.
Yes, please commit this suggestion: https://github.com/GenericMappingTools/pygmt/pull/1013/files#r589324327. After this gets merged, the transparency and overplotting can be dealt with in a separate PR. You are more than welcome to open the next PR for the gallery example or leave it at this important contribution of improving the colors and adding distinct symbols.
Yes, as @weiji14 mentioned the new colors are good. If you are OK with the suggestions, then you can click 'Files changed' in the PR and add the four suggested revisions to a batch commit using the 'Add suggestion to batch' option before committing them all. |
Ping @vitorgt to see if you have time to finalize this PR. |
Sorry for the late reply |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please move the example into the newly created gallery/symbols folder @vitorgt ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
🎉🎉🎉 Congrats on merging your first pull request and welcome to the team! 🎉🎉🎉 Please open a new pull request to add yourself to the |
…enericMappingTools#1013) Co-authored-by: Michael Grund <[email protected]>
#1011
I'm replying to an open good first issue (#1011), using color names defined in GMT docs as referred by @weiji14 (thanks).
Fixes #1011