-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Use Literal for statistics.quantiles() #7463
Conversation
This comment has been minimized.
This comment has been minimized.
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.
Looks good. I don't think #7258 needs to block this.
Diff from mypy_primer, showing the effect of this PR on open source code: core (https://github.com/home-assistant/core)
+ homeassistant/components/statistics/sensor.py:646: error: Argument "method" to "quantiles" has incompatible type "str"; expected "Union[Literal['inclusive'], Literal['exclusive']]" [arg-type]
|
Given that there are false positives in practice, "prefer false negatives over false positives" would imply we shouldn't do this without a fallback overload right? |
How would we do a fallback overload in this case? |
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 think this is a good change to make. The one false positive is very easily fixed. #7258 is a temporary moratorium on literal constants, not on literal argument types.
It'd be a bad fallback in that we'd have zero additional type safety but would still mean we get better IDE hover docs / completion docs :-) |
Shoot, I screwed up the commit description a bit there. Apologies :) |
It happens :) It's not super critical with stdlib stubs anyway. With third-party stubs, this matters more because commit messages turn into an auto-generated changelog. |
Fixes #7462