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

Improve help text of CLI options that use extend and ExtendOverwriteDefault actions #1654

Open
Tracked by #1642
joverlee521 opened this issue Oct 17, 2024 · 2 comments
Labels
documentation Improvements or additions to documentation

Comments

@joverlee521
Copy link
Contributor

Initially discussed in #1653 (comment).

The default behavior for the extend action in argparse is to extend the defaults. We have a custom ExtendOverwriteDefault action that overwrites the defaults instead of extending them, but this custom action does not change the output of the CLI help docs.

Comparing --metadata-delimiters which uses ExtendOverwriteDefault and --expected-date-formats which uses the default extend action from augur curate format-dates. The help text does not differentiate the extend vs overwrite behavior.

--metadata-delimiters METADATA_DELIMITERS [METADATA_DELIMITERS ...]
                        Delimiters to accept when reading a plain text metadata file. Only one delimiter will be inferred. (default: (',', '\t'))
--expected-date-formats EXPECTED_DATE_FORMATS [EXPECTED_DATE_FORMATS ...]
                        Expected date formats that are currently in the provided date fields, defined by standard format codes as listed at
                        https://docs.python.org/3/library/datetime.html#strftime-and-strptime-format-codes. If a date string matches multiple formats, it will be parsed as the
                        first matched format in the provided order. (default: ['%Y-%m-%d', '%Y-%m-XX', '%Y-XX-XX', 'XXXX-XX-XX'])
@joverlee521 joverlee521 added the documentation Improvements or additions to documentation label Oct 17, 2024
@victorlin
Copy link
Member

I think the current text is ambiguous for both ExtendOverwriteDefault and extend. How about something like this? (Note: properly rendering newline requires #1630)

 --metadata-delimiters METADATA_DELIMITERS [METADATA_DELIMITERS ...]
                         Delimiters to accept when reading a plain text metadata file. Only one delimiter will be inferred.
+                        Specified values will override the default list.
                         (default: (',', '\t'))
 --expected-date-formats EXPECTED_DATE_FORMATS [EXPECTED_DATE_FORMATS ...]
                         Expected date formats that are currently in the provided date fields, defined by standard format codes as listed at
                         https://docs.python.org/3/library/datetime.html#strftime-and-strptime-format-codes. If a date string matches multiple formats, it will be parsed as the
                         first matched format in the provided order.
+                        Specified values will extend the default list.
                         (default: ['%Y-%m-%d', '%Y-%m-XX', '%Y-XX-XX', 'XXXX-XX-XX'])

@victorlin
Copy link
Member

This makes me wonder why action="extend" is used over ExtendOverwriteDefault.

In all other usages besides --expected-date-formats, there is no default so it doesn't matter whether ExtendOverwriteDefault or extend is used and I think extend is used out of convenience.

For --expected-date-formats, the list ['%Y-%m-%d', '%Y-%m-XX', '%Y-XX-XX', 'XXXX-XX-XX'] might as well be hardcoded elsewhere in the code and the option be called something like --additional-expected-date-formats.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants