8000 [MNT]: _check_color_like for list like input · Issue #25005 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content
8000

[MNT]: _check_color_like for list like input #25005

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

Closed
story645 opened this issue Jan 16, 2023 · 2 comments
Closed

[MNT]: _check_color_like for list like input #25005

story645 opened this issue Jan 16, 2023 · 2 comments

Comments

@story645
Copy link
Member
story645 commented Jan 16, 2023

Summary

There's a private method that validates color input. This method is designed for scaler key value pairs, for example _check_color_like(color='green', gapcolor='orange')

def _check_color_like(**kwargs):
"""
For each *key, value* pair in *kwargs*, check that *value* is color-like.
"""
for k, v in kwargs.items():
if not is_color_like(v):
raise ValueError(f"{v!r} is not a valid value for {k}")

I think this validation should be extended to list like input, something like _check_color_like(color=['green', 'orange', 'yellow']) or a second private function _check_color_like_listlike(**kwargs) that supports that input.

Proposed fix

The trickiest part of the coding is probably in sorting out how to recognize RGBA tuples as a color and not a list. And that might be partially solved by making a second function that always calls the list version -> this requires looking through the code for uses of color (hint: look for to_rgba) and checking if the type is ambiguous at the conversion stage. (For example in pie and bar, the colors could be either but are shoved into a generator and could be validated using _check_color_like as part of that loop iteration).

This was motivated by #24849 to be consistent with color checking in #23208

It's a good first issue because it's private API around printing a good error message and is not touching the public 'is_color_like' method. But, I will also accept if the solution is always use the singular case and wrap it in list comprehensions as needed.

@story645 story645 added Good first issue Open a pull request against these issues if there are no active ones! topic: color/color & colormaps Maintenance labels Jan 16, 2023
@timhoffm
Copy link
Member

Please go for a separate explicit _check_color_like_list(**kwargs). This is an internal API and it's not worth the hassle distinguishing RGB(A) tuples from color lists.

@i-deal
Copy link
i-deal commented Jan 18, 2023

Hey, I'm a first-time contributor and I took a try at making the _check_color_like_list(**kwargs) function in the PR above. It does not try to distinguish between RGB(A) tuples, however, it handles color list inputs correctly. Do you think my suggestion should be modified in any way? Thank you.

@rcomer rcomer linked a pull request Jan 21, 2023 that will close this issue
3 tasks
@story645 story645 removed the Good first issue Open a pull request against these issues if there are no active ones! label Feb 2, 2023
@story645 story645 closed this as completed Feb 2, 2023
@story645 story645 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
0