| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
to_rgba signature and error message to optionally support the argument name Co-authored-by: i-deal <113572023+i-deal@users.noreply.github.com>
|
I am not at all excited about extending the API of to_rgba to add the ability to template the error message in a limited way. That is typically much better done via try:
c = to_rgba(...)
except ValueError as e:
raise ValueError("some custom and more helpful error message with context") from e # or from None
and we should not be leaking what is really internal details (helpers that existing to reduce the above boiler plate) into the public API. I am a bit worried that this passed tests as is_color_like has a special cut-out for the 'C2' style colors which can not be evaluated until after we have rcparams fullly setup. We should sort out if we no longer can ever try to do that, if we need to add a test , or be sure if the affected code paths can never be run during start up. The biggest worry here is adding a "fails to import" type bug which the user can not recover from! |
Sorry, something went wrong.
FWIW, both Patch and Collection go ahead and set the color as the rgba value, e.g. matplotlib/lib/matplotlib/patches.py Lines 317 to 341 in e9050e8 The original color is retained as a private attribute, but I think only used when updating alpha. Re the try-except loop: it seems to me that that is basically what _check_color_like is doing, neatly packaged so we don’t need to write it out in multiple places. |
Sorry, something went wrong.
to_rgba also has a carve out: matplotlib/lib/matplotlib/colors.py Lines 289 to 293 in e9050e8 And trying to pull the carve out of is_color_like broke the tests, so my guess is it's used in the startup stuff.
Yup, and I like it and was just unhappy that lists of colors was using to_rgba_array directly, but it's only used in 5 places and the multiple argument functionality is never used. Basically I just want consistency in color validation, also in where colors are validated, because aside from those I think everything else gets validated in draw time, and some of this has to do w/ inconsistencies in supported args like the auto/none thing happening here. there is also the decorator approach to hide the internal arg (which will also work for all the to_rgba functions) but I'm worried about runtime: def error_msg_parameter_name(f, c, errname=None):
try:
value = f(c)
except ValueError as e:
print(dir(e))
raise ValueError(str(e) +f'{"is not a valid value for " + errname if errname else "."}')
return value |
Sorry, something went wrong.
|
Can we clarify the status of this one? |
Sorry, something went wrong.
|
I've moved to draft. |
Sorry, something went wrong.
|
That's fine, honestly I'd prefer a more consistent approach to validation, probably as part of the rcParams work. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
In #25025 there was opposition to adding a new color checking method for arrays to be consistent with the _check_color_like checks, and in the call today there seemed to be consensus for using to_rgba as the validator, especially since _check_color_like is using it under the hood. _check_color_like was used in 5 places, so just swapped out those calls w/ calls to to_rgba. Didn't do anything w/ the return type mostly because I wasn't sure if we wanted class objects to retain the original input.
To keep the error messages from check_color_like, I went with the suggestion in this comment but didn't document it (yet) b/c I'm not sure if this is really a public keyword.
Originally posted by @anntzer in #25025 (comment)