Fix classvar typing doc#95773
Conversation
Remove redundant if check from optional argument function in argparser.
|
All commit authors signed the Contributor License Agreement. |
There was a problem hiding this comment.
Not sure this is an improvement since it makes it less obvious the difference between damage and stats. The reason the example isn't runnable or checkable as-is is because we've elided the def __init__(self, damage): self.damage = damage (presumably to keep things short). I'd be more in favour of adding that __init__ to the example than the current change.
oh yes, it make sense. Thanks. I will fix the PR. |
|
It’d probably be a good idea to keep the instance variable annotation outside |
| stats: ClassVar[dict[str, int]] = {} # class variable | ||
| damage: int = 10 # instance variable | ||
|
|
||
| def __init__(self, damage: int): | ||
| self.damage = damage # instance variable |
There was a problem hiding this comment.
We should still keep the instance variable type annotation at the class-level:
class Starship:
stats: ClassVar[dict[str, int]] = {} # class variable
damage: int = 10 # instance variable
def __init__(self, damage: int): ...
(would make a suggestion, but github doesn't let suggestions include deleted lines)
This is important because the confusion that ClassVar resolves is what the intention of a class-level annotation is
|
Sorry but I agree with @hauntsaninja and I don't think this is an improvement over the current docs. If anything it distracts from the example. |
|
Based on the responses to this PR, I'm adding the pending close label. |
|
I think the example is fine as is. Adding an |
|
Two typing maintainers are against this change. I'm closing the PR. |
Fix
classVartyping doc, fix the example.