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

Support Literal types as an alternative means of specifying choices #46

Merged
merged 8 commits into from
Mar 28, 2023

Conversation

lvdgraaff
Copy link
Contributor

This PR adds support for using a Literal type to specify a fixed number of argument choices. This feature is intended as a improvement over the use of the choices field of the metadata.

Example usage

from argparse_dataclass import dataclass
from typing import Literal


@dataclass
class Opt:
    z: Literal['a', 'b'] = 'a'


def main():
    params = Opt.parse_args()
    print(params)


if __name__ == "__main__":
    main()

Rationale

Specifying choices using a type rather than via the metadata field has two advantages.

  1. Using native Python functionality reduces the need of learning/looking up the options that this specific module provides. This improves readability and usability.
  2. Using a Literal type instead of a more generic type (int, float, str) for a limit set of options narrows the type specification and makes optimum use of the functionality of type checkers such as mypy.

Considerations

  1. The Literal type is supported from Python 3.8. As dropping support for earlier versions of Python is already anticipated on in Drop support for Python 3.6, 3.7 #39, no time was spend on testing this in older Python versions.
  2. The use of metadata['choices'] is still possible and has precedence over the use of the Literal type.

Content of PR

  1. Support for Literal type added to argparse_dataclass._add_dataclass_options()
  2. Test cases added to tests/test_argumentparser.py
  3. Update README.rst on the use of choices.

P.S. Thank you very much for developing and sharing this highly useful module! Highly appreciated.

Support Literal types as an alternative means of specifying choices.
Test cases for the Literal type
Copy link
Owner

@mivade mivade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks!

I think we'll probably want to check to avoid collisions between typing as Literal and providing choices. That case will also need a test to ensure proper behavior.

argparse_dataclass.py Show resolved Hide resolved
* Do not allow choices in the meta data when using a Literal type
* Add test case
@lvdgraaff
Copy link
Contributor Author

Hi Mike,

Thanks for you fast reply and enthusiasm. I just committed a small change to address your concerns. You're right that a collision between using both a Literal type and having choices in the Metadata should be prevented. In my opinion, even when they would match because that kind of redundancy is error prone.

But one might argue that it is desirable to allow the following:

@dataclass
class Options:
    a_or_b: Literal['a', 'b'] = field(metadata={choices : ['a', 'b']})

Now, I raise a ValueError when both are given. Even when valid. I also added a test case for that. What's your opinion on this?

@mivade
Copy link
Owner

mivade commented Mar 27, 2023

CI failures are due to black formatting. Once those are fixed this should be ready to merge.

@lvdgraaff
Copy link
Contributor Author

Apparently, there are small differences in handling white space lines between the VSCode Black plugin and the command line script :(.

@mivade
Copy link
Owner

mivade commented Mar 28, 2023

Thanks!

@mivade mivade merged commit 540640f into mivade:main Mar 28, 2023
3 of 5 checks passed
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.

None yet

2 participants