dates: Rewrite using regular expressions #928

victorlin · 2022-05-16T22:48:39Z

Description of proposed changes

This rewrites the date parsing functions currently used by filter and frequencies --min-date/--max-date parameters. Note that it doesn't affect the date parsing functions used for the metadata dates within augur filter.

Related issue(s)

Testing

Updated doctests and pytests.
Checks pass

codecov · 2022-05-18T17:21:04Z

Codecov Report

Base: 68.08% // Head: 68.23% // Increases project coverage by +0.14% 🎉

Coverage data is based on head (aeea545) compared to base (85d2edd).
Patch coverage: 96.82% of modified lines in pull request are covered.

❗ Current head aeea545 differs from pull request most recent head 3627df2. Consider uploading reports for the commit 3627df2 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #928      +/-   ##
==========================================
+ Coverage   68.08%   68.23%   +0.14%     
==========================================
  Files          62       62              
  Lines        6690     6727      +37     
  Branches     1641     1649       +8     
==========================================
+ Hits         4555     4590      +35     
  Misses       1829     1829              
- Partials      306      308       +2

Impacted Files	Coverage Δ
augur/dates.py	`93.69% <96.42%> (+0.45%)`	⬆️
augur/__version__.py	`100.00% <100.00%> (ø)`
augur/filter/__init__.py	`100.00% <100.00%> (ø)`
augur/frequencies.py	`51.66% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

augur/dates.py

j23414

Slick, especially in the 'any_to_numeric_type_min/max'.

As far as I can tell from reading the code, looks good to me.

augur/dates.py

victorlin · 2022-06-24T19:14:57Z

Converting to draft, at least until #989 is resolved

tsibley · 2022-06-24T19:37:54Z

Is the plan to eventually replace the remaining treetime-based functions like augur.date.get_numerical_dates() with the new ones introduced here?

victorlin · 2022-06-24T19:41:43Z

@tsibley yes, that's my plan with #938

augur/dates.py

To be more accurate and remove duplication.

victorlin · 2023-01-25T22:00:45Z

While re-reading through the commits, I realized it was hard to follow. Just rebased and force-pushed with more verbose commits which should hopefully help make sense of the changes here.

PR contents updated

One function per supported date format makes the top-level numeric_date() easier to follow.

This was previously implicit, but having it explicitly makes the flow more clear.

This rewrites the date parsing functions currently used by filter and frequencies --min-date/--max-date parameters. Note that current logic is retained.

Don't support non-positive years as those years are outside the range of the ISO 8601 standard.

Support both ambiguous dates (e.g. 2020-XX-XX) and incomplete dates (e.g. 2020-06, 2020).

By centralizing all calls to a final base case, this will make it easier to add options such as min/max bounds. It also makes usage of treetime.utils.numeric_date more clear.

This fix a bug¹ where incomplete dates such as --max-date 2018 would not be inclusive, since that had previously resolved to --max-date 2018-01-01. ¹ nextstrain#893

victorlin · 2023-01-28T00:22:00Z

Marking as draft again since this needs to go through some more changes following merge of #1140.

victorlin · 2023-01-31T01:22:06Z

Closing this in favor of #1144, which is an attempt to tackle the same problems while consolidating logic simultaneously.

victorlin self-assigned this May 16, 2022

victorlin force-pushed the dates/improve-parsing branch from f36b3f0 to f3ee9b9 Compare May 17, 2022 04:41

victorlin mentioned this pull request May 17, 2022

filter: re-implement with SQLite and add option to use with --engine #854

Closed

21 tasks

victorlin force-pushed the dates/improve-parsing branch 3 times, most recently from 49bf6ff to 215fb80 Compare May 18, 2022 17:10

victorlin force-pushed the dates/improve-parsing branch 2 times, most recently from e987104 to 505c7b9 Compare May 18, 2022 18:25

victorlin marked this pull request as ready for review May 18, 2022 18:36

victorlin requested a review from a team May 18, 2022 20:13

j23414 reviewed May 18, 2022

View reviewed changes

augur/dates.py Outdated Show resolved Hide resolved

victorlin mentioned this pull request May 18, 2022

dates: Consolidate date parsing logic #936

Open

7 tasks

j23414 previously approved these changes May 18, 2022

View reviewed changes

victorlin mentioned this pull request May 19, 2022

dates: Use same date parsing functions for both CLI arguments and metadata #938

Closed

3 tasks

victorlin force-pushed the dates/improve-parsing branch 3 times, most recently from 8d12b61 to 7fb35ac Compare May 28, 2022 00:50

victorlin mentioned this pull request Jun 18, 2022

Robust transform-date-fields nextstrain/mpox#45

Merged

tsibley self-requested a review June 22, 2022 22:42

tsibley previously requested changes Jun 22, 2022

View reviewed changes

victorlin mentioned this pull request Jun 24, 2022

dates: Clarify intended date format support by DateDisambiguator #989

Closed

victorlin marked this pull request as draft June 24, 2022 19:15

tsibley reviewed Jun 24, 2022

View reviewed changes

augur/dates.py Outdated Show resolved Hide resolved

victorlin force-pushed the dates/improve-parsing branch 3 times, most recently from 917dc79 to 6229db7 Compare July 1, 2022 18:16

fanninpm reviewed Jul 29, 2022

View reviewed changes

augur/dates.py Outdated Show resolved Hide resolved

victorlin force-pushed the dates/improve-parsing branch from 6229db7 to 53cb6c1 Compare November 8, 2022 22:57

Update docstrings

92fef32

To be more accurate and remove duplication.

victorlin force-pushed the dates/improve-parsing branch from 53cb6c1 to 9460e6c Compare January 25, 2023 21:59

victorlin force-pushed the dates/improve-parsing branch from aeea545 to 6a18b35 Compare January 25, 2023 23:03

victorlin marked this pull request as ready for review January 25, 2023 23:06

victorlin requested a review from a team January 25, 2023 23:06

victorlin added 11 commits January 26, 2023 17:26

Update comments to be more clear

5934909

Put date conversion logic in separate functions

fa152d1

One function per supported date format makes the top-level numeric_date() easier to follow.

Convert date to string

1073645

This was previously implicit, but having it explicitly makes the flow more clear.

Use InvalidDateFormat to handle user errors

67a442d

Rewrite date parsing with regular expressions

daa721c

This rewrites the date parsing functions currently used by filter and frequencies --min-date/--max-date parameters. Note that current logic is retained.

Scale back support for year-only dates and raise awareness of bug

962a3f1

Don't support non-positive years as those years are outside the range of the ISO 8601 standard.

Add support for ambiguous ISO dates

6e81f63

Support both ambiguous dates (e.g. 2020-XX-XX) and incomplete dates (e.g. 2020-06, 2020).

Make numeric_date recursive

c095930

By centralizing all calls to a final base case, this will make it easier to add options such as min/max bounds. It also makes usage of treetime.utils.numeric_date more clear.

filter/frequencies: handle ambiguity in --min-date and --max-date

2fc7b43

This fix a bug¹ where incomplete dates such as --max-date 2018 would not be inclusive, since that had previously resolved to --max-date 2018-01-01. ¹ nextstrain#893

Add pytests for various date formats

1178e4f

Use regexes in get_numerical_date_from_value()

3627df2

victorlin force-pushed the dates/improve-parsing branch from 6a18b35 to 3627df2 Compare January 27, 2023 01:35

victorlin marked this pull request as draft January 28, 2023 00:22

victorlin closed this Jan 31, 2023

victorlin deleted the dates/improve-parsing branch January 31, 2023 01:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dates: Rewrite using regular expressions #928

dates: Rewrite using regular expressions #928

victorlin commented May 16, 2022 •

edited

Loading

codecov bot commented May 18, 2022 •

edited

Loading

j23414 left a comment

victorlin commented Jun 24, 2022

tsibley commented Jun 24, 2022

victorlin commented Jun 24, 2022

victorlin commented Jan 25, 2023

victorlin commented Jan 28, 2023

victorlin commented Jan 31, 2023

dates: Rewrite using regular expressions #928

dates: Rewrite using regular expressions #928

Conversation

victorlin commented May 16, 2022 • edited Loading

Description of proposed changes

Related issue(s)

Testing

codecov bot commented May 18, 2022 • edited Loading

Codecov Report

j23414 left a comment

Choose a reason for hiding this comment

victorlin commented Jun 24, 2022

tsibley commented Jun 24, 2022

victorlin commented Jun 24, 2022

victorlin commented Jan 25, 2023

victorlin commented Jan 28, 2023

victorlin commented Jan 31, 2023

victorlin commented May 16, 2022 •

edited

Loading

codecov bot commented May 18, 2022 •

edited

Loading