fix: add missing overloads for argparse.*parse_*_args#8553
fix: add missing overloads for argparse.*parse_*_args#8553kkirsche wants to merge 2 commits intopython:masterfrom kkirsche:fix/8538
Conversation
This comment has been minimized.
This comment has been minimized.
|
Yikes, I must have messed this up 😕 . Maybe I should have left the original signature so that the overloads are just variants and the actual function has the correct signature? |
|
The overloads are correct (apart from I'm not sure what the best solution is. We could just leave the stubs as is: they would be incorrect, because the return type shouldn't always have parsed_args, remaining_args = parser.parse_known_args(namespace=MyNamespace())
assert isinstance(parsed_args, MyNamespace) |
|
My MyPy integration in VSCode was saying that not having it |
|
Yeah, the error I was seeing was:
|
|
The error is because without a bound, you could use |
|
Since we are using the variable to represent a namespace, why would we choose to avoid using |
|
There's no particular reason why the namespace would have to derive from class MyNamespace:
foo: list[str]
bar: int
args = parser.parse_args(namespace=MyNamespace())
print(args.foobar) # errorIf you inherit from |
|
Ah, ok, thank you. The namespace concept with argparse has been an abstraction that hadn't made sense to me in the past with argparse, so I appreciate the information about that. |
I think this may be too incorrect of a representation (as I understand it, a lot of people seem to be anti-union returns 😕 ), but I figure I'll ask just to ensure we've considered all options. What about something like this: |
|
Diff from mypy_primer, showing the effect of this PR on open source code: pytest (https://github.com/pytest-dev/pytest)
+ src/_pytest/config/argparsing.py:448: error: Incompatible return value type (got "Optional[Namespace]", expected "Namespace") [return-value]
materialize (https://github.com/MaterializeInc/materialize)
+ misc/python/materialize/mzcompose/__init__.py:900: error: Signature of "parse_known_args" incompatible with supertype "ArgumentParser"
+ misc/python/materialize/mzcompose/__init__.py:900: note: Superclass:
+ misc/python/materialize/mzcompose/__init__.py:900: note: @overload
+ misc/python/materialize/mzcompose/__init__.py:900: note: def parse_known_args(self, args: Optional[Sequence[str]] = ..., namespace: None = ...) -> Tuple[Namespace, List[str]]
+ misc/python/materialize/mzcompose/__init__.py:900: note: @overload
+ misc/python/materialize/mzcompose/__init__.py:900: note: def [_N] parse_known_args(self, args: Optional[Sequence[str]], namespace: _N) -> Tuple[_N, List[str]]
+ misc/python/materialize/mzcompose/__init__.py:900: note: @overload
+ misc/python/materialize/mzcompose/__init__.py:900: note: def [_N] parse_known_args(self, args: Optional[Sequence[str]] = ..., *, namespace: _N) -> Tuple[_N, List[str]]
+ misc/python/materialize/mzcompose/__init__.py:900: note: Subclass:
+ misc/python/materialize/mzcompose/__init__.py:900: note: def parse_known_args(self, args: Optional[Sequence[str]] = ..., namespace: Optional[Namespace] = ...) -> Tuple[Namespace, List[str]]
+ misc/python/materialize/cli/mzcompose.py:657: error: Signature of "parse_known_args" incompatible with supertype "ArgumentParser"
+ misc/python/materialize/cli/mzcompose.py:657: note: Superclass:
+ misc/python/materialize/cli/mzcompose.py:657: note: @overload
+ misc/python/materialize/cli/mzcompose.py:657: note: def parse_known_args(self, args: Optional[Sequence[str]] = ..., namespace: None = ...) -> Tuple[Namespace, List[str]]
+ misc/python/materialize/cli/mzcompose.py:657: note: @overload
+ misc/python/materialize/cli/mzcompose.py:657: note: def [_N] parse_known_args(self, args: Optional[Sequence[str]], namespace: _N) -> Tuple[_N, List[str]]
+ misc/python/materialize/cli/mzcompose.py:657: note: @overload
+ misc/python/materialize/cli/mzcompose.py:657: note: def [_N] parse_known_args(self, args: Optional[Sequence[str]] = ..., *, namespace: _N) -> Tuple[_N, List[str]]
+ misc/python/materialize/cli/mzcompose.py:657: note: Subclass:
+ misc/python/materialize/cli/mzcompose.py:657: note: def parse_known_args(self, args: Optional[Sequence[str]] = ..., namespace: Optional[Namespace] = ...) -> Tuple[Namespace, List[str]]
+ misc/python/materialize/cli/mzcompose.py:664: error: Incompatible return value type (got "Tuple[Optional[Namespace], List[str]]", expected "Tuple[Namespace, List[str]]")
+ misc/python/materialize/cli/mzcompose.py:668: error: Signature of "parse_known_args" incompatible with supertype "ArgumentParser"
+ misc/python/materialize/cli/mzcompose.py:668: note: Superclass:
+ misc/python/materialize/cli/mzcompose.py:668: note: @overload
+ misc/python/materialize/cli/mzcompose.py:668: note: def parse_known_args(self, args: Optional[Sequence[str]] = ..., namespace: None = ...) -> Tuple[Namespace, List[str]]
+ misc/python/materialize/cli/mzcompose.py:668: note: @overload
+ misc/python/materialize/cli/mzcompose.py:668: note: def [_N] parse_known_args(self, args: Optional[Sequence[str]], namespace: _N) -> Tuple[_N, List[str]]
+ misc/python/materialize/cli/mzcompose.py:668: note: @overload
+ misc/python/materialize/cli/mzcompose.py:668: note: def [_N] parse_known_args(self, args: Optional[Sequence[str]] = ..., *, namespace: _N) -> Tuple[_N, List[str]]
+ misc/python/materialize/cli/mzcompose.py:668: note: Subclass:
+ misc/python/materialize/cli/mzcompose.py:668: note: def parse_known_args(self, args: Optional[Sequence[str]] = ..., namespace: Optional[Namespace] = ...) -> Tuple[Namespace, List[str]]
+ misc/python/materialize/cli/mzcompose.py:675: error: Incompatible return value type (got "Tuple[Optional[Namespace], List[str]]", expected "Tuple[Namespace, List[str]]")
|
|
Looks like removing |
|
Yeah, it seems that we have multiple bad options to choose from. Feel free to try whatever you think might work and see what mypy_primer says :) Arguably returning the correct type when a namespace is passed isn't very important. Instead of this: You could just do this: Having to rewrite the code only to satisfy type checkers isn't great in general, but maybe it is the least bad option we have. |
Your suggestion was: def parse_known_args(
self, args: Sequence[str] | None = ..., namespace: Namespace | _N | None = ...
) -> tuple[Namespace | _N, list[str]]: ...Here returning There's one other problem with this definition. If you don't specify a from __future__ import annotations
from typing import TypeVar
T = TypeVar("T")
def func(x: str | T) -> T: ...
reveal_type(func(1)) # Revealed type is "builtins.int*"
reveal_type(func("hi")) # Revealed type is "<nothing>"When you try to assign the result to a variable, mypy will then complain about needing a type annotation for it, as it doesn't know what the type is (it's x = func("hi") # error: Need type annotation for "x"We probably don't want to require a type annotation for most uses of |
fix: #8538
This attempts to address issue #8538. Huge thank you to Akuli for their help in the issue. Hopefully I got this right, apologies if I messed anything up.