argparse: Use Literal for action & nargs add_argument parameter types by not-my-profile · Pull Request #7329 · python/typeshed

not-my-profile · 2022-02-20T18:41:19Z

As has previously been discussed in #6826 (comment) the argparse module sometimes compares argparse.SUPPRESS with is meaning Literal["==SUPPRESS=="] is not suited. This PR solves this using NewType.

As reported by mypy_primer the following fails with mypy:

import argparse
x = argparse.SUPPRESS
x = "test"

because of:

foo.py:4: error: Incompatible types in assignment (expression has type "str", variable has type "_SUPPRESS_T")

This however seems to be due to a limitation in mypy not understanding that a newtype is backwards compatible with the original type as can be observed in the following example:

import typing

X = typing.NewType('X', int)

def foo(a: X) -> int:
    return a

Mypy complains whereas Pyright says it's ok. With that in mind I think that the advantages of this PR (type safety against typos) outweigh the caused inconvenience (when reassigning SUPPRESS with mypy).

JelleZijlstra

Looks good. The cwltool code found by mypy-primer can just add explicit annotations.

Akuli · 2022-02-20T23:50:42Z

We could try SUPPRESS: _SUPPRESS_T | str to avoid the primer error. It's not really correct because SUPPRESS isn't just a string in the usual sense, but after foo = argparse.SUPPRESS you would at least be able to reassign a string.

github-actions · 2022-02-21T00:11:01Z

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

These were introduced in python#7329 and they cause false positives in code that used to be accepted before. There was a false positive in https://github.com/pycqa/pylint (encountered in python/mypy#12321) and I also saw false positives in an internal codebase. I suggest not using literal types here, since they can be kind of awkward to annotate in callers that don't pass a literal string. Rrequiring callers to write annotations like `Literal["?", "*", "+", "...", "A...", "==SUPPRESS=="]` is not very user friendly.

These were introduced in #7329 and they cause false positives in code that used to be accepted before.