MNT: Normalize internal set_foreground calls to RGBA - #31119
Conversation
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
|
Have you considered these two? matplotlib/lib/matplotlib/backend_bases.py Line 417 in 08fe8bc |
|
Thanks for the review @rcomer |
|
@timhoffm There has been couple of days since it open and ready for review could you? |
| if Nlinestyles: | ||
| gc0.set_dashes(*ls) | ||
| if len(ec) == 4 and ec[3] == 0.0: | ||
| ec_is_seq = ( |
There was a problem hiding this comment.
This function is looping through the properties of a Collection. Have you learned anything about Collection edgecolors that can help you simplify this?
There was a problem hiding this comment.
Yes Collection edgecolors are already RGBA via to_rgba_array, so I simplified the code to assume RGBA in the common case and kept a fallback for other inputs.
There was a problem hiding this comment.
Under what circumstances does the fallback get used?
There was a problem hiding this comment.
The fallback is only for non‑Collection callers that pass non‑RGBA values (e.g., color strings) into draw_path_collection/_iter_collection.
Collection always normalizes via mcolors.to_rgba_array, so the main path is RGBA. For other inputs we can’t safely assume RGBA, so we fall back to set_foreground(ec).
| else: | ||
| gc0.set_foreground(ec_rgba, isRGBA=True) | ||
| else: | ||
| gc0.set_foreground(ec) |
There was a problem hiding this comment.
This see seems a bit off. You do a whole song and dance to convert many cases to rgba, but then bail out. What is the motivation for this solution? Wouldn’t it be simpler to call to_rgba and be done?
There was a problem hiding this comment.
Thanks for the clarification chief!
|
Anyone can merge when CI is green (or as green as may be expected with the flaky tests). Recommend squash-merge. |
|
I took the liberty of unlinking the issue as I think there is more to do in terms of discouraging/deprecating public use of |
|
Question : When is matplotlib 3.11.0 is scheduled ? |
|
Soonish, but we need to get the font and text overhaul in first. #30161 |
|
By the way chief @timhoffm |
If we get accepted, it'll be under numfocus |
|
It means matplotlib come back this year for gsoc 2026 and have project ideas? |
If Numfocus gets accepted, then yes. You'll then find our project list linked through NumFocus. |
PR summary
This PR refactors internal calls to set_foreground within lib/matplotlib/ to ensure they pass normalized RGBA tuples with the isRGBA=True flag. This change aligns with the internal cleanup requested in issue #31105, reducing the overhead for backends to handle color normalization.
PR checklist