Implemented two helper functions lighter and darker which shade colors.#15596
Implemented two helper functions lighter and darker which shade colors.#15596chaoyi1 wants to merge 1 commit into
Conversation
|
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. |
|
This is based on my initiative and has two main use-cases:
|
jklymak
left a comment
There was a problem hiding this comment.
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.
| for i in range(len(v)): | ||
| if v[i] > 255: | ||
| s[i] -= v[i] - 255 | ||
| if s[i] < 0: |
There was a problem hiding this comment.
This whole block on v should be vectorized and the redundant conversion from/to 255 removed.
| "%(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 " |
There was a problem hiding this comment.
You seem tk have reformatted a lot of non relevant code.
| factor : float | ||
| any value above 1 means brightening the color, | ||
| any value in [0,1] means darkening the color | ||
|
|
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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), |
There was a problem hiding this comment.
I don’t see that any of these explicitly test the saturation wrapping when value exceeds 1
|
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 |
|
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 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 |
|
|
|
|
||
|
|
||
| def darker(color, factor): | ||
| """ |
There was a problem hiding this comment.
These don't appear in the rendered docs, that I can see....
There was a problem hiding this comment.
|
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. |
|
I'll close as not being actively worked on |
PR Summary
PR Checklist