Python options before algorithm - #3428
Merged
Merged
Conversation
A custom `_AlgorithmSelectAction` subclass of `argparse._SubParsersAction` is registered on every `Parser` instance. The default implementation parsed algorithm-specific arguments into a fresh namespace and then copied it over the master namespace, erasing values of any options set before the algorithm name. The replacement parses those arguments directly into the existing namespace with the algorithm parser's defaults temporarily suppressed, then applies any remaining defaults only to attributes not already set. Documentation for affected commands is updated to state that options may appear either before or after the algorithm name, and regression tests are added for both `5ttgen` and `dwi2response`. Prompt: > For Python commands where specific algorithms are invoked via subparsers, any command-line options specified before the subparser name are silently dropped. Investigate potential ways to preclude this behaviour. Consider ensuring that options specified before the algorithm name are suitably propagated to the sub-parser, or that they are parsed by the main parser and then merged with the content of the sub-parser, or detecting the presence of such options and issuing a warning to the user, or any other potential solutions. Investigate online for accepted best practise in use of the argparse package in this context. > > Worktree safe for modification; implement proposal for user review. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
With recent changes, it is no longer the case that any command-line options specified before the algorithm selector will be silently dropped. It is however not true that any command-line option can be placed anywhere on the command-line: while a member of the standard option group specified before the algorithm selector will be appropriately propagated through to the subparser, any algorithm-specific options will not, and will instead be treated as an unrecognised algorithm selection. Help text is therefore refined to say that command-line options *should* be placed after the algorithm name.
|
clang-tidy review says "All clean, LGTM! 👍" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An isolated cherry-picked change from a broader interrogation of the software CLI. From onset, for those Python commands that involve an algorithm selection, any command-line options specified before the algorithm selector could be silently dropped. This fixes that behaviour so that if a user pushes such a (non-recommended) usage, it will either fail to parse, or the command-line option will be propagated through to the sub-parser and therefore achieve the intended modulation of behaviour.