-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add a ...
initial value to attributes declared on object
's class
#13875
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
base: main
Are you sure you want to change the base?
Conversation
Diff from mypy_primer, showing the effect of this PR on open source code: vision (https://github.com/pytorch/vision)
- torchvision/utils.py:270: error: Unused "type: ignore" comment [unused-ignore]
|
While I'm not necessarily opposed to this, I think this should be discussed with the wider typing community. So far in typeshed, we've treated |
In fact, I don't think there's currently any way to really distinguish between these four cases in stubs: class X:
a: int
b: int
c: int = 3
def __init__(self) -> None:
self.b = 3
self.d = 4 |
We're also trying to detect this error in red-knot, and have also run into the issue that default values in class bodies are not generally included in stub files, unlike other Python source-code files. I'm basically in alignment with @srittau here -- I'm not necessarily opposed, but I do think some broader discussion might be good. This isn't just a typeshed-specific convention; I think most stubs outside typeshed also generally don't distinguish between instance attributes with class-level defaults and instance attributes without class-level defaults. The workaround red-knot has been using is to consider any annotations in stub files to constitute both a binding and a declaration (whereas in a non-stub file it would only constitute a declaration, not a binding). But if we can find consensus in the community that stubs should generally prefer to indicate whether an instance attribute has a class-level default or not, that would also benefit red-knot. |
Thank you so much for the feedback @srittau @AlexWaygood! I'll try to bring this up in the forum and follow red-knot's approach of special-casing these in stubs in the meantime. |
…lized Summary: See [this github issue](#123). Pyrefly currently complains on the following code: ``` class Foo: x: int Foo.x # ERROR! Instance-only attribute `x` of class `Foo` is not visible on the class ``` Problem is, typeshed doesn't really differentiate between uninitialized and initialized classlevel field. Meaning that this: ``` class object: __dict__: dict[str, Any] ``` and this: ``` class object: __dict__: dict[str, Any] = ... ``` are treated as equivalent (see [this PR](python/typeshed#13875) for further details). Based on the feedback of the PR, this is unlikely to be something that's going to change in the short term. And the suggested action was to avoid reporting the error when the containing class of the field comes from a stub file. Fixes #123 Reviewed By: rchen152 Differential Revision: D74035159 fbshipit-source-id: 72db88a9506294f4395d049549f3220e6fa2b4c1
Normally, attributes that are declared at class toplevel but are uninitialized there cannot be directly accessed from the class itself at runtime. For instance,
This makes sense because at runtime, the bare
x: int
"declaration" onFoo
toplevel is just an annotated assignment statement without a RHS, which semantically is a no-op. Nothing actually gets added to either the classFoo
or any of theFoo
object. But if we try to assign an initial value to the attribute,x
will be there:Pyrefly is making an attempt at reporting an error on this kind of issue, where if an attribute gets declared at class toplevel without an initial value, we ban all accesses to the attribute from the class object.
One issue we have, when enabling the error, is that many stub files on
typeshed
doesn't really pay much attention to the difference between initialized and uninitialized class toplevel attribute declarations. For example, there are several declarations inobject
class toplevel such as__doc__
and__dict__
where the value does actually exist on theobject
class directly, but typeshed models them as uninitialized class toplevel attribute which causes Pyrefly to misreport when those attributes are accessed.I don't know how far we could go with reporting this new kind of error (which neither mypy nor pyright would flag) eventually, but given that "fixing" the issue requires only trivial changes from the stub files, we may as well just try adding
= ...
to class-toplevel attributes that must exist at runtime, starting from the classobject
.2025-04-25 (@srittau): Deferred, pending further discussion with the wider typing community.