Skip to content

Use Literal types in argparse #6826

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

Closed
wants to merge 4 commits into from
Closed

Use Literal types in argparse #6826

wants to merge 4 commits into from

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Jan 5, 2022

No description provided.

@github-actions

This comment has been minimized.

Co-authored-by: Alex Waygood <[email protected]>
@sobolevn
Copy link
Member Author

sobolevn commented Jan 5, 2022

🤦

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

Diff from mypy_primer, showing the effect of this PR on open source code:

cwltool (https://github.com/common-workflow-language/cwltool)
+ cwltool/argparser.py: note: In function "arg_parser":
+ cwltool/argparser.py:460:51: error: Incompatible types in assignment (expression has type "Literal['Dependency resolver ']", variable has type "Literal['==SUPPRESS==']")  [assignment]
+ cwltool/argparser.py:464:13: error: Incompatible types in assignment (expression has type "Literal['Defaut root directory used by dependency resolvers configuration.']", variable has type "Literal['==SUPPRESS==']")  [assignment]
+ cwltool/argparser.py:466:34: error: Incompatible types in assignment (expression has type "Literal['Use biocontainers for tools without an ']", variable has type "Literal['==SUPPRESS==']")  [assignment]
+ cwltool/argparser.py:469:13: error: Incompatible types in assignment (expression has type "Literal["Short cut to use Conda to resolve 'SoftwareRequirement' packages."]", variable has type "Literal['==SUPPRESS==']")  [assignment]

@Akuli
Copy link
Collaborator

Akuli commented Jan 5, 2022

argparse is trickier than you might expect. It uses is to compare strings, so there is a difference between passing argparse.SUPPRESS and passing "==SUPPRESS==":

akuli@akuli-desktop:~$ python3 -c 'import argparse; p = argparse.ArgumentParser(); p.add_argument("--xyz", help=argparse.SUPPRESS); p.parse_args()' --help
usage: -c [-h]

optional arguments:
  -h, --help  show this help message and exit

akuli@akuli-desktop:~$ python3 -c 'import argparse; p = argparse.ArgumentParser(); p.add_argument("--xyz", help="==SUPPRESS=="); p.parse_args()' --help
usage: -c [-h] [--xyz XYZ]

optional arguments:
  -h, --help  show this help message and exit
  --xyz XYZ   ==SUPPRESS==

With this in mind, I'm not sure if Literal is the correct type for these constants. They really should be treated as if they were typing.NewType or something.

@sobolevn
Copy link
Member Author

sobolevn commented Jan 5, 2022

@Akuli thanks for the interesting use-case, I was not aware of that 🙂

But, I still think that Literal is the correct type here.
Because, I am not add_argument or help, I am just fixing constants to be in-line with https://typing.readthedocs.io/en/latest/source/stubs.html#constants

@Akuli
Copy link
Collaborator

Akuli commented Jan 8, 2022

But the constants really aren't meant to be treated as strings, but as special sentinel objects. Literal["foo"] means that you might as well pass the string "foo", and there should be no difference.

@JelleZijlstra
Copy link
Member

Closing per @Akuli's comment; there is also a merge conflict.

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 this pull request may close these issues.

4 participants