Do not check value of parameters from rcParam (and check rcParam befo… #26167
Conversation
| if style is None: | ||
| style = mpl.rcParams['font.style'] | ||
| _api.check_in_list(['normal', 'italic', 'oblique'], style=style) | ||
| else: | ||
| _api.check_in_list(['normal', 'italic', 'oblique'], style=style) |
There was a problem hiding this comment.
I'm not a fan of this kind of change. The code get's longer, harder to follow and more prone to errors.
Double checking the rcParam value here does not hurt. The only disadvantage are 300ns performance loss (which I claim to be negigible in any real-world application). But since the rcParam validation is an independent concept, we don't have a guarantee that they are synchronous. Such differences could be an oversight. But I can also imagine cases where rcParams are used in multiple places and have a slightly broader acceptance range than what a specific function supports.
There was a problem hiding this comment.
The thing is that those 300 ns are multiplied with a huge number in many situations.
There was a problem hiding this comment.
What is huge? If huge * 300ns > 2% of the total plot time, we may start to consider, otherwise, the benefit is negligible.
…re using them)
Inspired by some changes in #26164, but separated as they are not directly connected to that (some that could have been here might have gone in there though...).
If we get the rcParam (and that is checked) or set an argument to a valid value, it is not required to check with
check_in_list.Some rcParams were not checked when loaded, only when used.
Finally, small methods where the likely thing is that the correct parameter is passed and there are if-claused anyway, are checked at the end rather than the beginning.
PR summary
PR checklist