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

Support incomplete ambiguous dates #631

Merged
merged 5 commits into from
Nov 12, 2020

Conversation

huddlej
Copy link
Contributor

@huddlej huddlej commented Nov 10, 2020

Description of proposed changes

This PR continues work from #623 by adding support for incomplete ambiguous date strings like those matching the formats of "YYYY" and "YYYY-MM".

Related issue(s)

Related to #602.

Testing

Adds new unit tests for the case of incomplete ambiguous date strings.

Prior to merge

This PR should be rebased onto master after #623 is merged to avoid a messy git history.

@codecov
Copy link

codecov bot commented Nov 10, 2020

Codecov Report

Merging #631 (2ac2223) into master (748abc8) will increase coverage by 0.14%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #631      +/-   ##
==========================================
+ Coverage   28.53%   28.68%   +0.14%     
==========================================
  Files          39       39              
  Lines        5389     5389              
  Branches     1327     1326       -1     
==========================================
+ Hits         1538     1546       +8     
+ Misses       3793     3785       -8     
  Partials       58       58              
Impacted Files Coverage Δ
augur/filter.py 44.23% <33.33%> (+0.67%) ⬆️
augur/utils.py 36.44% <100.00%> (+1.95%) ⬆️

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 748abc8...2ac2223. Read the comment docs.

Reverts logic in augur filter to not consider ambiguous date results and
removes logic for tracking ambiguous dates from `get_numerical_dates`.
Updates the help documentation for the ambiguous date filter and uses
the "not" operator instead of "is False" for readability of boolean
expressions.
Expands the logic of the date ambiguity test function to support date
strings like "YYYY" and "YYYY-MM" that caused the previous
implementation to break. Since a date with only a year or year and month
is stil valid, we mockup ambiguous day and month values for these
incomplete date strings and explicitly test the resulting year, month,
and day values.

This commit also adds unit tests to represent these incomplete and
ambiguous date strings.
@huddlej huddlej force-pushed the support-incomplete-ambiguous-dates branch from 9ee8661 to 3fd75d8 Compare November 12, 2020 17:31
Cleans up unit tests for the `is_date_ambiguous` function, collecting
them into a single function with clearer documentation. Also, adds tests
for valid incomplete date strings and fixes a bug in the
`is_date_ambiguous` function that was revealed by these new tests.
@huddlej huddlej marked this pull request as ready for review November 12, 2020 17:45
Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

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

Nice work @huddlej - another super useful addition to augur 👍

tests/test_utils.py Show resolved Hide resolved
tests/test_utils.py Show resolved Hide resolved
tests/test_utils.py Show resolved Hide resolved
augur/utils.py Outdated Show resolved Hide resolved
augur/utils.py Outdated Show resolved Hide resolved
augur/filter.py Outdated Show resolved Hide resolved
Replaces the "all" option for `is_date_ambiguous` with "any" and updates
the help documentation, ambiguity boolean logic, and unit tests to
clarify this hierarchical treatment of ambiguity.
@huddlej huddlej merged commit 55348f9 into master Nov 12, 2020
@huddlej huddlej deleted the support-incomplete-ambiguous-dates branch November 12, 2020 23:17
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.

2 participants