correct parse_args namespace attribute#2566
correct parse_args namespace attribute#2566srittau merged 9 commits intopython:masterfrom PrajwalM2212:argparse
Conversation
stdlib/2and3/argparse.pyi
Outdated
| add_help: bool = ...) -> None: ... | ||
| def parse_args(self, args: Optional[Sequence[_Text]] = ..., | ||
| namespace: Optional[Namespace] = ...) -> Namespace: ... | ||
| namespace: Optional[object] = ...) -> object: ... |
There was a problem hiding this comment.
This is not the right solution. There are basically three cases that can be distinguished using @overload:
- No
namespaceargument given -> returnNamespace namespaceisNone-> returnNamespacenamespaceis any other value: return the same type. This can be achieved using aTypeVar.
srittau
left a comment
There was a problem hiding this comment.
This looks much better. Two issues, though.
stdlib/2and3/argparse.pyi
Outdated
| _N = TypeVar('_N') | ||
| @overload | ||
| def parse_args(self, args: Optional[Sequence[_Text]] = ..., | ||
| namespace: Type[_N] = ...) -> _N: ... |
There was a problem hiding this comment.
You need to remove the default from the namespace parameter here. Otherwise type checkers get confused when calling parse_args([]), because both overloads match.
Also, the namespace passed in is returned unchanged, not used as a constructor. So the annotation for namespace should be just _N, not Type[_N].
There was a problem hiding this comment.
Addendum: If you remove the default here, you should not need the third overload you just added.
There was a problem hiding this comment.
When I remove the default by setting namespace: _N , pycharm high-lightens it as non-default parameter follows default parameter. Should I ignore it ?
There was a problem hiding this comment.
Ah, my bad. Try these two lines instead of the one above:
@overload
def parse_args(self, args: Optional[Sequence[_Text]], namespace: Type[_N]) -> _N: ...
@overload
def parse_args(self, *, namespace: Type[_N]) -> _N: ...
stdlib/2and3/argparse.pyi
Outdated
|
|
||
| @overload | ||
| def parse_args(self, args: Optional[Sequence[_Text]] = ..., | ||
| namespace: Optional[Namespace] = ...) -> Namespace: ... |
There was a problem hiding this comment.
I think this overload shouldn't have the namespace argument. If namespace is given, the second overload suffices.
|
Closing ! Will reopen this when I understand more about how overload works in mypy |
|
@srittau @JelleZijlstra Thanks for your generous help. I will reopen this when I understand more about how overload works in mypy. |
|
@PrajwalM2212 Thank you for your effort to fix those small bugs, I really appreciate it! I think you were nearly there with this PR. Could you try this: _N = TypeVar('_N')
@overload
def parse_args(self, args: Optional[Sequence[_Text]] = ...) -> Namespace: ...
@overload
def parse_args(self, args: Optional[Sequence[_Text]], namespace: _N) -> _N: ...
@overload
def parse_args(self, *, namespace: _N) -> _N: ...This changes two things compared to your last version:
|
|
I am sorry that it failed again. It seems that pytype does not like typevars inside classes. I didn't know that. So just move it to the start of the file and then it should finally pass! Thank you for your persistence. |
fixes #2366
This PR allows
parse_argsnamespacearg to accept any object and also to return any object