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

Adjust grayscale color ramp #1353

Merged
merged 2 commits into from
Jun 5, 2021
Merged

Adjust grayscale color ramp #1353

merged 2 commits into from
Jun 5, 2021

Conversation

trvrb
Copy link
Member

@trvrb trvrb commented Jun 4, 2021

The existing grayscale color ramp (used for values absent in an explicitly specified color scale) had values that were too dark and threw off the overall color balance. This commit narrows the grayscale color ramp to be more in line with pastel color ramp.

I mainly did this because I didn't like the aesthetics of the new clades coloring. Here it is on current Auspice master:

master

And here is this PR:

PR

@trvrb trvrb requested a review from jameshadfield June 4, 2021 00:35
@jameshadfield jameshadfield temporarily deployed to auspice-grayscale-ramp-sbczwlz June 4, 2021 00:35 Inactive
The existing grayscale color ramp (used for values absent in an explicitly specified color scale) had values that were too dark and threw off the overall color balance. This commit narrows the grayscale color ramp to be more in line with pastel color ramp.
@trvrb trvrb force-pushed the grayscale-ramp branch from 1ef0bc6 to f506932 Compare June 4, 2021 01:10
@trvrb trvrb temporarily deployed to auspice-grayscale-ramp-sbczwlz June 4, 2021 01:10 Inactive
This adds a bit of blue into the grayscale color ramp. Still reads as mostly gray, but no colors seem to exist more in the same universe as canonical auspice color ramp.
@trvrb trvrb temporarily deployed to auspice-grayscale-ramp-sbczwlz June 4, 2021 22:20 Inactive
@trvrb
Copy link
Member Author

trvrb commented Jun 4, 2021

I've updated this PR to inject a bit of color into the "grayscale" color ramp. I'm done this for both extra values not explicitly defined in the categorical color ramp, and also for the "unassigned" values (?, etc...). I'm rather pleased with the results, which seem to place these "null" colors more in the same universe as the default pastel color ramp.

injected-color

@jameshadfield: This is ready to merge from my perspective.

Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

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

Happy for these changes to go through ✅

In the future we may want to better distinguish between "extra values" (i.e. defined traits which weren't assigned a colour by the dataset) and "unknown values":

auspice_greys

P.S. It's not this simple, as auspice doesn't render hex values truthfully for the tree. We make branch colours closer to grey, brighten terminal nodes and brighten legend swatches by a different (!) amount. I believe colours in the frequencies panel & the map are rendered truthfully, but I may be wrong. Here's a side-by-side of current auspice (where we lighten things) and a version where we don't:

image

@jameshadfield jameshadfield merged commit d1801b1 into master Jun 5, 2021
@jameshadfield jameshadfield deleted the grayscale-ramp branch June 5, 2021 00:10
@trvrb
Copy link
Member Author

trvrb commented Jun 5, 2021

Distinguishing between Auspice assigned colors for traits that don't have an explicit listing and traits that are "unassigned" is a good argument to keep unknownColor at #AAAAAA.

We could have a situation that involves both kinds of grays and I can see it being useful to distinguish.

I didn't think about all the lightening we've been doing. I'd probably aim for center of tip bubble to be the exact color that's specified and have the center of the legend swatch also be exact color specified. We then lighten the edge of the tip bubble and the edge of legend swatch by the same amount.

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