- Notifications
You must be signed in to change notification settings - Fork673
fix(cli): allow exclusive arguments as optional#2770
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
fix(cli): allow exclusive arguments as optional#2770
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedJan 21, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #2770 +/- ##======================================= Coverage 96.48% 96.49% ======================================= Files 90 90 Lines 5866 5874 +8 =======================================+ Hits 5660 5668 +8 Misses 206 206
Flags with carried forward coverage won't be shown.Click here to find out more.
|
9a16bdd
tod9d260c
CompareWould it make more sense to create a mutually exclusive group and add the exclusive args to that? https://docs.python.org/3/library/argparse.html#mutual-exclusion |
I've changed the code to create a mutually exclusive group. I think the data model of RequiredOptional is not ideal. Whether options are exclusive should be perpendicular to whether they are required or optional. Also, it would be possible to have multiple exclusive groups. Currently this is not reflected in the RequiredOptional.exclusive property. It only allows one group, and it is not clear whether this group is required or not. Ideally, you would have something lilke this:
But adding these exclusive options to the CLI argument parser at least makes it functioning again, if not perfect. |
24dc41e
toa53397d
CompareUh oh!
There was an error while loading.Please reload this page.
The CLI takes its arguments from the RequiredOptional, which has three fields: required, optional, and exclusive. In practice, the exclusive options are not defined as either required or optional, and would not be allowed in the CLI. This changes that, so that exclusive options are also added to the argument parser.Closespython-gitlab#2769
e435fc9
to04171d0
CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thank you very much for this.
The CLI takes its arguments from the RequiredOptional, which has three fields: required, optional, and exclusive. In practice, the exclusive options are not defined as either required or optional, and would not be allowed in the CLI. This changes that, so that exclusive options are also added to the argument parser.
Closes#2769
Changes
Documentation and testing
Please consider whether this PR needs documentation and tests.This is not required, but highly appreciated: