Update docs for curate format-dates
#1653
Conversation
| @@ -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. | |||
There was a problem hiding this comment.
[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?
There was a problem hiding this comment.
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'])
There was a problem hiding this comment.
[
--metadata-delimiterscf.--expected-date-formats] Neither the help nor the docs differentiates the extend vs overwrite behavior
Perfect example right there!
There was a problem hiding this comment.
Tracking separately in #1654
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1653 +/- ##
=======================================
Coverage 72.26% 72.26%
=======================================
Files 79 79
Lines 8271 8272 +1
Branches 1691 1691
=======================================
+ Hits 5977 5978 +1
Misses 2009 2009
Partials 285 285 ☔ View full report in Codecov by Sentry. |
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`.
joverlee521
force-pushed
the
format-dates-docs
branch
from
56aa236
to
c7e8a31
Compare
|
Rebased onto master and fixed merge conflicts in the changelog. |
Description of proposed changes
Improve docs for
curate format-dates(docs preview)See commits for details.
Related issue(s)
Resolves #1651
Checklist