-
Notifications
You must be signed in to change notification settings - Fork 0
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 color vision deficiency, gradient, and palette display tools #20
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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 did a first pass. Looks pretty good; just the usual comments about naming and clarity.
Some general comments:
- the names
simulate_color
,simulate_color_all
, andsimulate_colors
are all very similar. I made some suggestions in inline comments about this, but one way or another, I think it's something we'd want to make a bit clearer (even if these are primarily meant for power users). In particular, I would make sure the names distinguish between methods that return simulated colors and methods that print/display simulated colors. - For specifying color deficiencies, I would favor a more specific argument name than
form
likecvd_type
. - the docstrings of
simulate_palette
andsimulate_palette_all
are duplicated, as are the docstrings forsimulate_gradient
andsimulate_gradient_all
. - In the
cvd
module, I would make thecvd_types
dict a global and use it to instead of repeating the hard-coded list of['d', 'p', 't']
in the subsequent methods. - should
Palette
also have areverse
method? - The type hints are incomplete (I flagged some of these, but not all).
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.
Very close, a couple suggestions + changes
@@ -58,6 +63,64 @@ def swatch(self, steps=21): | |||
|
|||
return "".join(swatches) | |||
|
|||
def reverse(self): | |||
return Gradient( | |||
name=f"{self.name}_r", |
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.
Maybe reverse
instead of r
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.
Here, I'm trying to conform with matplotlib
's syntax for designating reverse gradients. So, I think we should keep this in place.
|
||
def reverse(self): | ||
return Palette( | ||
name=f"{self.name}_r", |
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.
same here
PR checklist
Fixes #123, Resolves #456
).Overview
This PR adds functionality for simulating color vision deficiency under the
cvd
module.HexCode
objects), palettes, and gradients undercvd.simulate_color
,cvd.simulate_palette
, andcvd.simulate_gradient
. These methods return a transformed version of the color.cvd.simulate_color_all
,cvd_simulate_palette_all
, andcvd.simulate_gradient_all
. These methods returnNone
but display all 4 color spaces (normal, d, p, and t).cvd.simulate_gradient_lightness()
to display gradient lightness for all three major types of color vision deficiency.In addition, this PR adds some helpful utilities to the
Palette
andGradient
classes:swatch()
method forPalette
, which displays all the colors in the palette as a single strip of swatches prior to the specific color swatches, which has parallel functionality to theswatch()
method forGradient
..display_all()
method under each of thepalettes
andgradients
modules, which displays a summary of all of the available colormaps within each module, as per Add function to display all palettes/ gradients #16.reversed()
method forGradient
s, as per Add ability to reverse and slice gradients #17.matplotlib
's list of color gradients, with the suffix_r
, as is conventional formatplotlib
resample()
method forGradient
s, which returns aPalette
with a resampled list of colors from the originalGradient
, allowing users to useGradient
colors for a categorical plotting purpose, as requested in Create function that returns resampled HEX codes from gradient #4.interpolate()
method forGradient
s, which returns an updatedGradient
with the values of colors adjusted so that they have uniformly increasing lightness inCAM02-UCS
colorspace, addressing part of Add ability to reverse and slice gradients #17.Testing
I confirmed that these functions work using
usage_example.ipynb
, where you can also see how these methods are meant to be used.Not addressed
We are still finalizing the gradients, palettes, and font sizes, etc.; those changes will come in a future PR, including better names for unidirectional and bidirectional gradients. We're also working on making sure we have standardized sizing/ naming for plot sizes which can be drag/drop placed into Illustrator templates.
I also have not written tests for the
mpl
module or this new functionality yet... would love some help to do this if anyone has bandwidth.Halp
Would love feedback on the naming and structure of things! I think we can be generally afford to be more verbose / less intuitive with some of the
cvd
stuff, since it's meant to be used primarily by color-obsessives like myself and Audrey.