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

Experience report with some suggestions and a bit of a rant. #16599

Open
markshannon opened this issue Dec 1, 2023 · 1 comment
Open

Experience report with some suggestions and a bit of a rant. #16599

markshannon opened this issue Dec 1, 2023 · 1 comment
Labels

Comments

@markshannon
Copy link
Member

markshannon commented Dec 1, 2023

Experience report

I recently attempted to use MyPy for the first time in a while and found it frustrating.
My perception was that MyPy was quite poor, being difficult to use and with quite a number of false positives.

<rant>

MyPy it is quite poor with the default settings. It produces a lot of false positives, with no obvious way to fix them, as they aren't real issues.

</rant>

Suggestions

MyPy is a powerful tool and quite capable, but it does not present itself as such by default.
The defaults need to be selected to improve the user experience by minimizing false positives and friction.

MyPy is a type checker, not a general static analysis tool. It has static analysis capabilities, but they should be off by default.
Nagging the user about style issues, when they want type checking is not a good experience.

For example, allow_redifinition and implicit_reexport should be on by default.
Whichever option best matches Python semantics should be the default.

False positives

False positives are bad. They distract from real errors, reduce trust in the results of the type checker and will discourage users.

It seems silly to me that so many false positives are a result of poor defaults, not hard-to-fix bugs.

Strict versus "lax"

The --strict option seems to be treated as a "very fussy" option and reports all sorts of "errors" such as missing annotations where none are needed.
Both --strict and "lax" should report errors where there are known errors, obviously ("lax" might choose to not report errors in some cases).
--strict should also report errors when the lack of annotations means that it cannot prove that there is no error. It should not report errors merely from a lack of annotations or the wrong settings.

For example, this code is correct, no error should be reported:

def print_sum(seq: list[int]):
    total = 0
    for i in seq:
        total += i
    print(total)

The return type is trivially None, as there is no return statement. No need for an annotation.

But this code is obviously wrong:

def returns_int():
    return 0
    
def greet():
    return "Hello " + returns_int()

I'm not sure if "lax" should complain here. PEP 484 says it shouldn't, but it is clearly wrong code.
--strict should report an error, obviously. Not only that, but it should report the real error, i.e. the type error in "Hello " + returns_int(), not the lack of annotation on returns_int()

Too many options

MyPy is a type checker. A program is either type safe, or it isn't.
It might be impossible to decide if it is type safe, but adding options isn't going to help.
For each option, choose the one that gives the right answer and throw away the other one.

Summary

There is a good tool in MyPy made to look bad by poor choice of defaults.
It shouldn't be hard to fix them and make it look good.

MyPy is a type checker not a linter. Hide the linting options behind --lint or similar so that people can use them if they want, but they shouldn't get in the way of type checking.

@AlexWaygood
Copy link
Member

allow_redifinition and implicit_reexport should be on by default.

implicit_reexport is on by default. It's only disabled in your case because you're running mypy with --strict, but you can still override that in the config file, with e.g.

strict = True
implicit_reexport = True

(I know that one of your other points is that the defaults for --strict should themselves be better; this is just responding to your specific point that "implicit_reexport should be on by default" :)

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

2 participants