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

Fix parse fix dates (take 2) #878

Merged
merged 3 commits into from
Apr 4, 2022
Merged

Fix parse fix dates (take 2) #878

merged 3 commits into from
Apr 4, 2022

Conversation

huddlej
Copy link
Contributor

@huddlej huddlej commented Mar 30, 2022

Description of proposed changes

An alternate solution to #874 that prefers renaming the parse_sequence argument from fix_dates to fix_dates_format to fix #873.

As part of this fix, I also noticed undesirable behavior from the fix_dates function (which we wouldn't have noticed, since the function hasn't been getting called!) where users are warned about valid ambiguous dates (e.g., "2020-XX-XX").

Related issue(s)

Fixes #873
Related to #874

Testing

This PR adds functional tests for the original issue and unit tests for the second issue and then adds the code to make these tests pass.

huddlej added 3 commits March 30, 2022 14:17
Updates functional tests for the parse command to use the `--fix-dates`
argument and updates the corresponding initial sequence data to include
a sequence with an incomplete date value that needs to be fixed. This
functional test fails, recapitulating issue #873.

Also, updates the unit tests for the `fix_dates` function to explicitly
check for warning messages when users provide invalid dates. These tests
now include an example where a user may provide a valid ambiguous date
that currently produces a warning and shouldn't.
Renames the `fix_dates` argument of `parse_sequence` to
`fix_dates_format`. This name is a slightly more accurate description of
the variable and also prevents the overwriting of the `fix_dates`
function by a function argument.

Fixes #873
Checks whether dates that could not be parsed by pandas are still valid
ambiguous dates for Augur (e.g., "2020-XX-XX") and does not warn users
about these ambiguous dates. Also, prints warnings to stderr where users
should expect to find them instead of stdout.
@huddlej huddlej mentioned this pull request Mar 30, 2022
@codecov
Copy link

codecov bot commented Mar 30, 2022

Codecov Report

Merging #878 (43ea88b) into master (751549e) will decrease coverage by 0.52%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master     #878      +/-   ##
==========================================
- Coverage   34.28%   33.75%   -0.53%     
==========================================
  Files          41       41              
  Lines        5942     6213     +271     
  Branches     1521     1673     +152     
==========================================
+ Hits         2037     2097      +60     
- Misses       3823     3999     +176     
- Partials       82      117      +35     
Impacted Files Coverage Δ
augur/parse.py 64.55% <80.00%> (+2.05%) ⬆️
augur/utils.py 43.83% <0.00%> (+0.44%) ⬆️
augur/export_v2.py 13.62% <0.00%> (+2.68%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 751549e...43ea88b. Read the comment docs.

@huddlej huddlej requested a review from rneher March 31, 2022 17:48
@huddlej huddlej merged commit 8387c31 into master Apr 4, 2022
@huddlej huddlej deleted the fix-parse-fix-dates branch April 4, 2022 17:27
@victorlin victorlin added this to the Next release X.X.X milestone Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

augur parse --fix-dates doesn't work
3 participants