Skip to content

More permissive behavior when overriding class variables #5639

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

Closed
msullivan opened this issue Sep 18, 2018 · 4 comments
Closed

More permissive behavior when overriding class variables #5639

msullivan opened this issue Sep 18, 2018 · 4 comments
Assignees
Labels

Comments

@msullivan
Copy link
Collaborator

The following code fails to typecheck:

class A:
    lol = None

class B(A):
    lol = 'b'

with
test.py:5: error: Incompatible types in assignment (expression has type "str", base class "A" defined the type as "None")

My proposal for fixing this is to add a new flag, on by default, that will cause class variables with the type None to instead be given the type Optional[Any].

Not sure what the flag should be called, though.

(I'm investigating top causes of errors when running mypy on an unannotated code base and this issue caused 60% of the 600+ errors generated when checking django.)

@msullivan msullivan added needs discussion priority-0-high false-positive mypy gave an error on correct code labels Sep 18, 2018
@msullivan msullivan self-assigned this Sep 18, 2018
@JelleZijlstra
Copy link
Member

For what it's worth, this was also one of the most common causes of errors when running mypy on our big unannotated codebase.

@emmatyping
Copy link
Member

emmatyping commented Sep 19, 2018

This does seem like a common pattern. However, I don't think this flag should be on by default. I'd rather have people who are starting anew turn something on over subtly changing the behavior for existing mypy users (and losing type safety they think exists).

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 19, 2018

This is a tricky issue. To make it easier to start using mypy, we could perhaps generalize the concept of strictness levels, which are presets for certain sets of options. We already have one, --strict, but we could easily add more. Here's one idea:

  • By default use strictness level 1 (say, equivalent to --strictness=1). This corresponds to the current behavior.
  • Provide a new level 0 ( --strictness=0) that turns on the proposed change and things like --ignore-missing-imports. Maybe this would also turn on --follow=imports=skip. This may be useful for projects with a lot of legacy code that just want to get a basic level of checking with minimal effort. We can introduce additional flags in the future that will be enabled by this option.
  • --strict could be equivalent to --strictness=5 or something.

We can introduce --strictness=0 early on in the documentation, but we'd recommend against using it unless the volume of mypy errors is high.

We can later add additional strictness levels, but the above three would probably be sufficient for now.

@ilevkivskyi
Copy link
Member

I think this was already discussed before, but I would also disable Need type annotation for variable for --strictness=0.

msullivan added a commit that referenced this issue Sep 25, 2018
A major cause of false positives when checking new codebases is
unannotated top level containers and Nones.

Add a flag that suppresses both the "needs type annotation" errors and
the assigning of NoneTyp to variables at the "toplevel" (not when
nested in functions). To avoid throwing away too much type
information, we give more precise types in these situations than
previously, filling in the arguments to the type with `Any`.

Closes #5639.
msullivan added a commit that referenced this issue Sep 25, 2018
A major cause of false positives when checking new codebases is
unannotated top level containers and Nones.

Add a flag that suppresses both the "needs type annotation" errors and
the assigning of NoneTyp to variables at the "toplevel" (not when
nested in functions). To avoid throwing away too much type
information, we give more precise types in these situations than
previously, filling in the arguments to the type with `Any`.

Closes #5639.
msullivan added a commit that referenced this issue Sep 26, 2018
…5670)

A major cause of false positives when checking new codebases is
unannotated top level containers and Nones.

Add a flag that suppresses both the "needs type annotation" errors and
the assigning of NoneTyp to variables at the "toplevel" (not when
nested in functions). To avoid throwing away too much type
information, we give more precise types in these situations than
previously, filling in the arguments to the type with `Any`.

Closes #5639.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants