Repository navigation
Deprecate most ValidateInStrings validators. - #16483
timhoffm merged 1 commit into
Conversation
Sorry, something went wrong.
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?
Sorry, something went wrong.
|
I did not carefully check that the lists of strings were copied correctly. |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
(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 [_case_insensitive_marker_object, "blah", "blah"] which I think would still be an improvement readabililty-wise -- the marker object wouldn't leak out anyways as it'd be immediately consumed by the conversion to ValidateInStrings.) |
Sorry, something went wrong.
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?).
PR Summary
PR Checklist