Skip to content

Implemented two helper functions lighter and darker which shade colors.#15596

Closed
chaoyi1 wants to merge 1 commit into
matplotlib:masterfrom
chaoyi1:lighter-and-darker
Closed

Implemented two helper functions lighter and darker which shade colors.#15596
chaoyi1 wants to merge 1 commit into
matplotlib:masterfrom
chaoyi1:lighter-and-darker

Conversation

@chaoyi1

@chaoyi1 chaoyi1 commented Nov 2, 2019

Copy link
Copy Markdown
Contributor

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@QuLogic

QuLogic commented Nov 2, 2019

Copy link
Copy Markdown
Member

Is there some actual use-case for these? I don't see anything using them, and I'm not sure we want to be a general-purpose colour-processing library.

@timhoffm

timhoffm commented Nov 3, 2019

Copy link
Copy Markdown
Member

This is based on my initiative and has two main use-cases:

  1. Creating matching shades of color for fill color and edge color, e.g. to easily create https://matplotlib.org/devdocs/gallery/text_labels_and_annotations/fancytextbox_demo.html#sphx-glr-gallery-text-labels-and-annotations-fancytextbox-demo-py based on any color

  2. Creating similar colors for related data, e.g. if I have datasets a_before, a_after, b_before, b_after I could

plot(a_after)
plot(a_before, color=lighter('C0', factor=1.5))
plot(b_after)
plot(b_before, color=lighter('C1', factor=1.5))

@jklymak jklymak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I’m a little skeptical about this as a feature as well. We already profvide to_hsv. Do we need this extra helper? I didn’t know about the wrapping saturation algorithm, so that is useful and maybe makes this worthwhile.

Comment thread lib/matplotlib/colors.py
for i in range(len(v)):
if v[i] > 255:
s[i] -= v[i] - 255
if s[i] < 0:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This whole block on v should be vectorized and the redundant conversion from/to 255 removed.

Comment thread lib/matplotlib/colors.py
"%(since)s and will be removed %(removal)s; please "
"use lowercase instead.")
"single-letter colors is deprecated since Matplotlib "
"%(since)s and will be removed %(removal)s; please "

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You seem tk have reformatted a lot of non relevant code.

Comment thread lib/matplotlib/colors.py
factor : float
any value above 1 means brightening the color,
any value in [0,1] means darkening the color

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please explain exactly what factor does. Otherwise it’s hard to guess what values to use and what they are doing to the color.

([1,1,1], 0.5)
])
def test_darker(color, factor):
from PyQt5.QtGui import QColor

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You don’t really need this dependency here, so I’d rewrite the test using numbers.

([1, 0, 0], 1.1),
([1, 1, 1], 3),
([0.4, 0.3, 0.1], 1.04),
([0.1, 0.1, 0.1], 1.03),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don’t see that any of these explicitly test the saturation wrapping when value exceeds 1

@ImportanceOfBeingErnest

Copy link
Copy Markdown
Member

It seems there is no standard package for colors in python; is that correct? Because ideally, such color arithmetics would be provided by a standard library and in turn matplotlib would provide an interface in the sense of

import python_colors as colors
plt.plot(..., color=colors.Color("red").darker())

@tacaswell

Copy link
Copy Markdown
Member

This has been discussed for while (dating back to 2013):

#2159, #2745, #8895, #9985, #9995

and a pretty well up-voted SO question https://stackoverflow.com/questions/37765197/darken-or-lighten-a-color-in-matplotlib . So there is clearly some desire for this among our users (and I suspect that a non-zero number of our users abuse alpha for this).

There exists several packages specific to color changes (https://colorspacious.readthedocs.io/en/latest/, https://docs.python.org/3.7/library/colorsys.html, https://github.com/vaab/colour) and we are in the process of standing up a new plotting-specific color library (https://github.com/pyviz/contrib_colormaps) . I think that this functionality would be a much better fit for one of those libraries.

I think a PR adding a (dependencyless) way of consuming the colour objects in our internal color normalization code would be a good thing.

However, I still think that the consensus from #9985 still holds that we don't want do this piecemeal and it looks like since that discussion at least one color specific package has arisen.

Thank you for your work @chaoyi1 , but I am not in favor of merging this as-is. I would rather see the colour integration above and a good example showing how to use it for the use-cases @timhoffm mentioned above.

@timhoffm

timhoffm commented Nov 5, 2019

Copy link
Copy Markdown
Member

colorspacious and pyviz/contrib_colormaps have different scopes. Only colour seems to be a possible color representation package. But I have two concerns with it:

  • I would expect such a package to accept the same colors as we do. Otherwise, there'll be a lot of confusion that I can do Color('blue').lighter() but not Color('mediumslateblue').lighter(). This is something that could be worked upon, but needs coordination with an external library (and their willingness to accept these). In particular alpha and CSS3 colors are currently not supported (RGBA support vaab/colour#42).
  • The colour package hasn't seen any activity for two years. I would only be willing to introduce support for such a library if long-term support is guaranteed. That would seem safer under a matplotlib or pyviz umbrella organization compared to a single-developer project.

Comment thread lib/matplotlib/colors.py


def darker(color, factor):
"""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These don't appear in the rendered docs, that I can see....

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@tacaswell

Copy link
Copy Markdown
Member

This was discussed on this weeks call: https://paper.dropbox.com/doc/In-Matplotlib-2019-meeting-agenda--AoeL8YHb2wWKxPGdvfQ0hkolAg-aAmENlkgepgsMeDZtlsYu#:h2=color-manipulation-tools

The consensus we came to is that we do want this functionality, but want to think a bit more about the algorithm and parameters. We should do some research to see if there is a "standard" way to do this across CSS / Qt / ... and go with that rather than inventing our own thing.

@jklymak

jklymak commented Apr 23, 2021

Copy link
Copy Markdown
Member

I'll close as not being actively worked on

@jklymak jklymak closed this Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants