-
Notifications
You must be signed in to change notification settings - Fork 334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make ConfigSetting generic #3215
Conversation
db14098
to
f1882f2
Compare
0a8e11c
to
403500f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this! I was holding off from adding generics, since this has changed so much from our lower bar of 3.9 to now, that I didn't think it was worth it, but looks like it already works somewhat sufficiently. Nice!
One thing I forgot: This should probably be checked on Python 3.9 to ensure this doesn't introduce anything by chance that fails on 3.9. |
Mypy and pyright both assume Python 3.9 in CI |
bbd21a7
to
72eb7a9
Compare
Yeah, but type checking and actually running without throwing an exception (e.g. just the help output), are two different things, unfortunately. We've had plenty of cases where successfully type-checked code crashed because of circular imports right at import time. :) |
This prevents one from accidentally using incompatible parse/match/default_factory callbacks. A few type ignores are necessary due to python/mypy#3737 and in
finalize_config
because any inference is impossible there