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

Update docs for curate format-dates #1653

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

joverlee521
Copy link
Contributor

@joverlee521 joverlee521 commented Oct 17, 2024

Description of proposed changes

Improve docs for curate format-dates (docs preview)
See commits for details.

Related issue(s)

Resolves #1651

Checklist

  • Automated checks pass
  • Check if you need to add a changelog message
  • Check if you need to add tests
  • Check if you need to update docs

Make it clear that the incomplete dates are not automatically masked.
The user has to explicitly provide the incomplete date formats.
Because of the added default values in
<#1501>, this is no longer a
required argument.

The alternative is to explicitly mark this option as `required=True`,
but that's a breaking change that can be considered later. Currently,
if `--expected-date-formats` is not provided and dates match the
defaults, then it's a no-op. If dates do not match the default formats,
it will raise a loud error so the user can add the custom formats.
When a date parsing failure occurs, show the current expected date
formats with the hint that they can be updated
via `--expected-date-formats`.
@@ -29,6 +30,7 @@ This is expected to fail with an error, so redirecting stdout since we don't car
ERROR: Unable to format dates for the following (record, field, date string):
(0, 'collectionDate', '2020-01')
(0, 'releaseDate', '2020-01')
Current expected date formats are ['%Y-%m-%d', '%Y-%m-XX', '%Y-XX-XX', 'XXXX-XX-XX', '%Y', '%Y-%m-%dT%H:%M:%SZ']. This can be updated with --expected-date-formats.
Copy link
Member

Choose a reason for hiding this comment

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

[aside] Oh - this test output really helped me understand the --expected-date-formats argument - it doesn't overwrite the defaults, it extends them. That wasn't really clear from the help / docs. Do you know if there is there an established pattern for conveying whether an argument extends vs replaces the defaults?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, now that you mention it, I don't think this is clear in any of the help / docs.

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. Neither the help nor the docs differentiates 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'])

Screenshot 2024-10-17 at 12 17 10 PM
Screenshot 2024-10-17 at 12 17 21 PM

Copy link
Member

Choose a reason for hiding this comment

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

[--metadata-delimiters cf. --expected-date-formats] Neither the help nor the docs differentiates the extend vs overwrite behavior

Perfect example right there!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tracking separately in #1654

Copy link

codecov bot commented Oct 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.24%. Comparing base (862aa37) to head (56aa236).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1653   +/-   ##
=======================================
  Coverage   72.24%   72.24%           
=======================================
  Files          79       79           
  Lines        8268     8269    +1     
  Branches     1691     1691           
=======================================
+ Hits         5973     5974    +1     
  Misses       2009     2009           
  Partials      286      286           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Update help docs for augur curate format-dates
2 participants