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

argparse.ArgumentParser.parse_args() overload misfire #2641

Closed
gvanrossum opened this issue Nov 27, 2018 · 6 comments
Closed

argparse.ArgumentParser.parse_args() overload misfire #2641

gvanrossum opened this issue Nov 27, 2018 · 6 comments
Labels
stubs: false negative Type checkers do not report an error, but should

Comments

@gvanrossum
Copy link
Member

I had some code calling parse_args(args, ns) where the type of ns was Optional[Namespace. It then deduced that the return type was also Optional[Namespace]. I think this is a mistake -- I don't think parse_args() ever returns None. Perhaps _N should be constrained by bound='Namespace'?

@srittau
Copy link
Collaborator

srittau commented Nov 27, 2018

The argparse documentation gives an explicit example of a non-Namespace namespace argument. Personally, I prefer a false negative over a false positive, especially in this case.

@gvanrossum
Copy link
Member Author

OK, but can we at least do something so that mypy understands that parse_args(args, None) does not return None? Surely that's a special case worth handling. My example code is roughly

def parse_args_wrapper(argv: List[Text], namespace: Optional[Namespace]) -> Namespace:
    return ...parse_args(argv, namespace)

and this gives an error on the return saying that it expects Namespace but got an Optional[Namespace] -- I can only explain this by asuming that this is because _N is unconstrained in the second and third overloads.

Maybe adding another overload with an explicit None will help?

@srittau
Copy link
Collaborator

srittau commented Nov 27, 2018

Explicit None overloads are definitely missing, but would not help your use case, as far as I can tell, since that case would not match. Best idea I have is an overload for Optional[_N] -> Any, where Any is really Union[Namespace, _N]. (Another case the AnyOf (python/typing#566) would be useful.)

@gvanrossum
Copy link
Member Author

Hm, I'm not so sure. Here's a simple test program:

from argparse import ArgumentParser, Namespace
from typing import Optional

def foo(p: ArgumentParser, ns: Optional[Namespace]) -> Namespace:
    return p.parse_args(namespace=ns)

This currently gives

t.py:5: error: Incompatible return value type (got "Optional[Namespace]", expected "Namespace")

But when I insert

    @overload
    def parse_args(self, *, namespace: None) -> Namespace: ...

in the list of parse_args() overloads (before the last one), it passes without errors.

(We'd also need an extra overload for (Optional[Sequence[_Text]], None) -> Namespace.)

@srittau
Copy link
Collaborator

srittau commented Nov 27, 2018

Huh, learned something new today. That's definitely useful and in that case just overloading None seems like the correct fix.

@srittau srittau added size-small stubs: false negative Type checkers do not report an error, but should labels Nov 27, 2018
@ilevkivskyi
Copy link
Member

Union math kicks in soon on purpose, mypy now works much better with overloads :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: false negative Type checkers do not report an error, but should
Projects
None yet
Development

No branches or pull requests

3 participants