swansonk14 / typed-argument-parser

Typed argument parser for Python
MIT License
494 stars 40 forks source link

support for ignoring args #115

Closed rainyl closed 6 months ago

rainyl commented 1 year ago

Just do the same thing as #75 and #92 , but meet the format mentioned at https://github.com/swansonk14/typed-argument-parser/pull/92#issuecomment-1435766167

for now, args can be ignored as follow:

# test.py

from tap import Tap, TapIgnore

class MyTapWithIgnore(Tap):
    arg_to_ignore_by_comment: str  # tap: ignore
    attr_to_ignore: TapIgnore[str]
    classvar_attr: TapIgnore[str]
    hello: str = "hello"

args = MyTapWithIgnore().parse_args()

print(args)

and the output:

python test.py -h
usage: test.py [--hello HELLO] [-h]

options:
  --hello HELLO  (str, default=hello)
  -h, --help     show this help message and exit
python test.py
{'hello': 'hello'}

By the way, some codes were borrowed from #92 and the official typing library.

martinjm97 commented 1 year ago

Hi @rainyl,

Thank you for contributing! We appreciate you adding TapIgnore.

Prior to merging, we have a few concrete recommendations and questions.

For subscriptable types in a TapIgnore, the IDE does not seem to know that argignore: TapIgnore[int] means that argignore is of type int. Rather, it thinks it is of type Any. Is there a way to make this work and if not what's a good solution?

Screenshot 2023-08-02 at 3 48 20 PM

Ideally, there'd be a test case then ensures that TapIgnore typed arguments cannot be provided on the command line.

We're not familiar with the _SpecialForm. Is this the standard way to define a subscriptable type?

The current code seems to fail a test: ERROR tests/test_actions.py - TypeError: __init__() missing 1 required positional argument: 'doc'

We'll do our best to help out, but would love further contributions. Thank you again!

Best, Kyle and Jesse

rainyl commented 1 year ago

Hi, @martinjm97

Well, after working for some time, I used some hacking techniques to implement it on python version >=3.9, for 3.8 and former, the best solution seems to be using ClassVar directly.

TL;DR, regard TapIgnore as an alias to ClassVar, but with a name of "TapIgnore"

TapIgnore = ClassVar
TapIgnore._name = "TapIgnore"

Specifically, a user-defined type TapIgnored[T] different from types in typing lib will be regard as class and can't be treated as T directly, this feature must be supported by type checking library like pyright (or pylance, mypy, etc.) to extract specific T, you know, it's nearly impossible, at least for now. In the above code, hacking means it's not a standard and elegant implementation.

Finally, I think the best implementation is that: for python 3.9+, use above solution. for python 3.8-, give up supporting TapIgnore[T] -> T, use Generic[T] instead, hint in IDE will be shown as TapIgnored[int]

Also, If you have better solutions, please let me know :)

rainyl commented 1 year ago

Ideally, there'd be a test case then ensures that TapIgnore typed arguments cannot be provided on the command line.

As for this, I find it had been solved.

For example, for the following code

from tap import Tap, TapIgnore

class MyTapWithIgnore(Tap):
    arg_to_ignore_by_comment: str  # tap: ignore
    attr_to_ignore: TapIgnore[str]
    hello: str = "hello"

args = MyTapWithIgnore().parse_args([
    "--hello", "hello world",
    # "--attr_to_ignore", "ignored",
    # "--arg_to_ignore_by_comment", "ignored by comment"
])

print(args)

and the output:

(venv38) > python .\test.py
usage: test.py [--hello HELLO] [-h]
test.py: error: unrecognized arguments: --attr_to_ignore ignored --arg_to_ignore_by_comment ignored by comment
(venv38) > python .\test.py
usage: test.py [--hello HELLO] [-h]
test.py: error: unrecognized arguments: --attr_to_ignore ignored
(venv38) > python .\test.py
{'hello': 'hello world'}
martinjm97 commented 1 year ago

Current implementation. It seems that the current solution (on Python 3.10) improves the situation by making it so that the type is TapIgnore[int] rather than Any.

Screenshot 2023-09-06 at 4 44 08 PM

Ideally, we'd want it to be of type int.

Suggestion. The code below does seem to work for us (at least for Python 3.10)!

T = TypeVar('T')
TapIgnore: TypeAlias = ClassVar[T]
TapIgnore._name = 'TapIgnore'

What do you think? We set the _name field in the code above in the same way you did in your code. Why did you add that line? It doesn't seem to affect the behavior.

Thank you for keeping on working on this. We're very grateful.

Best, Kyle and Jesse

rainyl commented 1 year ago

Well, the codes you provided above won't work in VSCode.

T = TypeVar('T')
TapIgnore: TypeAlias = ClassVar[T]
TapIgnore._name = 'TapIgnore'

image

but this worked:

TapIgnore = ClassVar
TapIgnore._name = "TapIgnore"

image

However, this solution seems won't work in pycharm, which has it's own type checking tool.

As for TapIgnore._name="TapIgnore", It is because that, the name of TapIgnore with Tapignore=ClassVar will be ClassVar rather than Tapignore, and thus, the result of get_origin() will ClassVar.

Finally, I suggest that just use ClassVar to indicate ignored variables, the tricks above may not work in every type checking tool like pylance, mypy, and pycharm's, but they all support ClassVar.

martinjm97 commented 12 months ago

Hi @rainyl,

Thank you again for the follow up! What an interesting problem. We weren't able to exactly reproduce the issue you observed, but we had a similar problem with VSCode until we installed mypy.

For Python 3.10.0, we used your test case above and looked at results in PyCharm and VSCode with TapIgnore defined as follows:

T = TypeVar('T')
TapIgnore: TypeAlias = ClassVar[T]
TapIgnore._name = 'TapIgnore'

VSCode version 1.82.1 Without mypy installed, it doesn't identify problems in type checking as you observed. However, with mypy installed, it seemed to work (it gave type TapIgnore[str] rather than str, but it still correctly identified the type mismatch and said it was between str and int rather than TapIgnore[str] and int). This isn't ideal, but it clears the bar for us in terms of practicality. We can add mypy as a dependency to resolve this problem for users.

Pycharm version 2023.1.1 (Community Edition) Everything worked as expected.

Let us know if installing mypy works for you as well.

Thank you again, JK (Jesse and Kyle)