Skip to content
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

reportUninitializedInstanceVariable: a false positive, and a false negative #4687

Closed
AlexWaygood opened this issue Feb 24, 2023 · 1 comment
Labels
as designed Not a bug, working as intended

Comments

@AlexWaygood
Copy link

AlexWaygood commented Feb 24, 2023

Describe the bug

reportUninitializedInstanceVariable appears to emit false-positive errors for subclasses of argparse.Namespace, and does not emit errors where it should for classes decorated with @dataclass(init=False).

To Reproduce
Create the following Python file in an empty directory, and run pyright on it:

# pyright: reportUninitializedInstanceVariable=error
import argparse
import dataclasses


class Args(argparse.Namespace):
    x: int  # Pyright reports an error here


parser = argparse.ArgumentParser()
parser.add_argument("--x", action="store_true")
args = parser.parse_args([], namespace=Args())
y = args.x  # No exception at runtime


@dataclasses.dataclass(init=False)
class Bar:
    x: int  # Pyright does *not* report an error here


b = Bar()
y = b.x  # Exception at runtime

Here is the output if you execute this Python file:

Traceback (most recent call last):
  File "c:\Users\alexw\coding\pyright_repro\test.py", line 22, in <module>
    y = b.x  # Exception at runtime
        ^^^
AttributeError: 'Bar' object has no attribute 'x'

Expected behavior

No error should be reported for the Args class, because it is not expected for subclasses of argparse.Namespace to define __init__ methods. The primary reason (the only reason I know of) to subclass argparse.Namespace is to create custom classes of which instances can be passed to argparse.ArgumentParser.parse_args. It would be unidiomatic for subclasses of argparse.Namespace to define __init__ methods, since attributes on Namespace objects are dynamically set in the call to argparse.ArgumentParser.parse_args(). The only reason why the instance variable is being declared at all in the above example is to provide a modicum of type safety: otherwise, the inferred type of args.x will be Any, which is undesirable.

The false positive can be suppressed with a type: ignore or a pyright: ignore, but this is pretty tedious, and very ugly, if you have an argparse.Namespace subclass with many attributes:

class Args(argparse.Namespace):
    a: int  # pyright: ignore[reportUninitializedInstanceVariable]
    b: int  # pyright: ignore[reportUninitializedInstanceVariable]
    c: int  # pyright: ignore[reportUninitializedInstanceVariable]
    d: int  # pyright: ignore[reportUninitializedInstanceVariable]
    e: int  # pyright: ignore[reportUninitializedInstanceVariable]

In python/typeshed#9793, we encountered this problem, and found a workaround for this issue: replace the argparse.Namespace with an init=False dataclass, since using a dataclass provides similar behaviour to subclassing argparse.Namespace. In other words, we applied this diff:

+ @dataclass(init=False)
+ class Args:
- class Args(argparse.Namespace):
      x: int

  parser = argparse.ArgumentParser()
  parser.add_argument("--x", action="store_true")
  args = parser.parse_args([], namespace=Args())
  y = args.x

However, while this was a useful workaround to avoid the false-positive for argparse.Namespace subclasses, it illustrates that reportUninitializedInstanceVariable does not provide safety guarantees for dataclasses that don't have __init__ methods. This is a false negative in its own right: pyright should probably treat @dataclass(init=False) classes just like ordinary classes that don't have __init__ methods when it comes to this check.

@erictraut
Copy link
Collaborator

erictraut commented Feb 24, 2023

Yes, the reportUninitializedInstanceVariable check results in many known false positives and negatives. For that reason, it is not enabled by default even in strict type checking mode, so if it doesn't suit your use case, you can leave it disabled.

I don't think it's worth trying to plug all of the holes by adding more special-casing logic. I'm more inclined to just remove the feature altogether because it's not useful to all but a tiny percentage of pyright users. I think I'll leave it in place for now, but I don't plan to invest more into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended
Projects
None yet
Development

No branches or pull requests

2 participants