Skip to content

Add overloads for argparse.ArgumentParser.parse_x_args() #8538

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

Open
NCPlayz opened this issue Aug 12, 2022 · 7 comments
Open

Add overloads for argparse.ArgumentParser.parse_x_args() #8538

NCPlayz opened this issue Aug 12, 2022 · 7 comments

Comments

@NCPlayz
Copy link

NCPlayz commented Aug 12, 2022

An extension of #2566, seems like the typings for parse_known_args, parse_intermixed_args, parse_known_intermixed_args (and the undocumented _parse_known_args) of argparse.ArgumentParser were overlooked, as they all share the same pattern as parse_args for typing the namespace class.

@kkirsche
Copy link
Contributor

kkirsche commented Aug 15, 2022

That file also seems to not exist anymore in that location, but parse_known_args and whatnot are documented here it seems:

So it looks like it's a matter of updating these with the proper overloads

@kkirsche
Copy link
Contributor

Corresponding Python source:

Due to the argument ordering and the way the methods return (always a list[str] but the namespace can be generic or not, I'm not 100% sure how to write the overloads properly, so I'll have to defer to a typeshed team member to provide guidance or assist in fixing this, if possible.

@NCPlayz
Copy link
Author

NCPlayz commented Aug 15, 2022

parse_args passes through to parse_known_args, so I think the typing for the namespace arg would be the exact same as that method.

I think the return type is fine as tuple[_N, list[str]]? But I could be wrong.

@kkirsche
Copy link
Contributor

kkirsche commented Aug 15, 2022

The issue I'm not sure how to properly handle in the overloads is related to having an argument without a default after an argument with a default value as we only need to overload the namespace, based on my review of the source code's behavior (unless I misread it). You're correct about the return type though, I'm just not sure how to represent it within the mypy's rules for argument orders and default values unless it's correct to just leave those out of the overloads.

@Akuli
Copy link
Collaborator

Akuli commented Aug 15, 2022

an argument without a default after an argument with a default value

It would be tempting to write this:

@overload
def parse_args(self, args: Iterable[str] | None = ..., namespace: None = ...) -> Namespace: ...
@overload
def parse_args(self, args: Iterable[str] | None = ..., namespace: _T) -> _T: ...

But as you noticed, the second overload invalid syntax. We can split it into two overloads based on how the namespace argument is passed. It is either passed as a positional argument, in which case args is also positional and must be provided:

@overload
def parse_args(self, args: Iterable[str] | None, namespace: _T) -> _T: ...

Or as a keyword argument, here args can be positional or keyword:

@overload
def parse_args(self, args: Iterable[str] | None = ..., *, namespace: _T) -> _T: ...

The * before namespace marks the remaining arguments (in this case just namespace) as keyword-only, so a call like parser.parse_args(args, my_namespace) won't match that overload. That's why we need the overload for positional arguments.

Putting it all together, we get three overloads:

@overload
def parse_args(self, args: Iterable[str] | None = ..., namespace: None = ...) -> Namespace: ...
@overload
def parse_args(self, args: Iterable[str] | None, namespace: _T) -> _T: ...
@overload
def parse_args(self, args: Iterable[str] | None = ..., *, namespace: _T) -> _T: ...

This question comes up quite regularly. Presumably we could document it somewhere instead of explaining it in a comment every time :)

@kkirsche
Copy link
Contributor

Thank you, @Akuli for the great explanation and apologies for the delay on getting back to this.

I've created a merge request with my understanding. Apologies if I got any aspect of it wrong. The one difference I left between the example you provided and the current stub was I left the use of Sequence rather than Iterable for args.

In terms of documentation, where would be the correct place to submit a merge request to document this? Happy to take a stab at it based on what you've shared.

Thanks again

@Akuli
Copy link
Collaborator

Akuli commented Aug 18, 2022

CONTRIBUTING.md already has sections about how to write stubs, such as "Stub versioning" and "Incomplete stubs". I think we could add a new section "Optional arguments before required arguments" or similar.

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

Successfully merging a pull request may close this issue.

3 participants