Repository navigation
Deprecate most ValidateInStrings validators. - #16483
Conversation
tacaswell
left a comment
tacaswell
left a comment
There was a problem hiding this comment.
👍 in principle, it does read much better.
The _fix_validators() which works by mutating global state as a side effect is a bit annoying, but I don't see a better option.
Maybe delete the function after we call it and leave a comment about why we need to have it?
|
I did not carefully check that the lists of strings were copied correctly. |
We can just list the allowed string values in the defaultParams dict (which is actually clearer) and then dynamically generate the corresponding validators; no need to expose them as separate public API (which is not particularly useful anyways because how can you know that it's `validate_fontset` for `mathtext.fontset`, but `validate_mathtext_default` for `mathtext.default`?).
|
Actually it's not too hard to do without the helper function (and without leaking any temporary variables), it's just a dict comprehension. Edited accordingly. |
|
(I didn't touch the case-insensitive ones yet because I haven't decided the best way to handle them, but it could be as simple as |
We can just list the allowed string values in the defaultParams
dict (which is actually clearer) and then dynamically generate
the corresponding validators; no need to expose them as separate
public API (which is not particularly useful anyways because how can
you know that it's
validate_fontsetformathtext.fontset, butvalidate_mathtext_defaultformathtext.default?).PR Summary
PR Checklist