Break utils.ambiuous_date_to_date_range to new class

[elebow]

Description of proposed changes

Rewrite utils.ambiguous_date_to_date_range as a new class.

This slightly expands the function's capabilities: any X character anywhere in the string will be disambiguated, rather than just at the least significant positions. It should be fully backwards-compatible.

Related issue(s)

none

Testing

Unit tests included.

[codecov]

Codecov Report

Merging #532 into master will increase coverage by 0.46%.
The diff coverage is 94.11%.

@@            Coverage Diff             @@
##           master     #532      +/-   ##
==========================================
+ Coverage   20.91%   21.38%   +0.46%     
==========================================
  Files          31       32       +1     
  Lines        5136     5159      +23     
  Branches     1302     1303       +1     
==========================================
+ Hits         1074     1103      +29     
+ Misses       4010     4004       -6     
  Partials       52       52              

Impacted Files	Coverage Δ
augur/util_support/date_disambiguator.py	93.75% <93.75%> (ø)
augur/utils.py	28.08% <100.00%> (-1.94%)	⬇️

---

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 f50e8f0...4f6f238. Read the comment docs.

[tsibley]

👍 for rewriting this!

This slightly expands the function's capabilities: any X character anywhere in the string will be disambiguated, rather than just at the least significant positions. It should be fully backwards-compatible.

However, I'm not sure that this behaviour expansion is desirable. 20X0 or 2020-XX-05 both seem to me more likely to be bogus data, not accurately vague time period data. I think folks doing analyses with Augur would be less surprised if 2020-XX-05 (for example) was rejected instead of taken at face value. That is, it seems more likely to be a data entry/curation error (perhaps intended to be 2020-05-XX) than a real data point considering the meaning of "The 5th of some month in 2020." vs. "May 2020". Consider also that ISO 8601 allows YYYY and YYYY-MM but not YYYY--DD. :-)

[emmahodcroft]

I agree with @tsibley on this!

[elebow]

Makes sense. I'll continue the legacy behavior here and open a followup issue to discuss possible enhancement.

[elebow]

Actually, I misread the legacy implementation when I wrote that note about expanding the functionality—Augur already behaves this way (except for a detail regarding the year).

# on current master branch

from augur.utils import ambiguous_date_to_date_range

>>> ambiguous_date_to_date_range("2000-01-01", "%Y-%m-%d")
(datetime.date(2000, 1, 1), datetime.date(2000, 1, 1))

>>> ambiguous_date_to_date_range("2000-01-XX", "%Y-%m-%d")
(datetime.date(2000, 1, 1), datetime.date(2000, 1, 31))

>>> ambiguous_date_to_date_range("2000-XX-05", "%Y-%m-%d")
(datetime.date(2000, 1, 5), datetime.date(2000, 12, 5))


# when year is uncertain, it falls back to the final (optional) argument:

>>> ambiguous_date_to_date_range("20XX-03-05", "%Y-%m-%d")
(None, None)

>>> ambiguous_date_to_date_range("20XX-03-05", "%Y-%m-%d", min_max_year=(1992, 2003))
(datetime.date(1992, 3, 5), datetime.date(2003, 3, 5))

I suppose there are three paths forward:

1. Keep the PR as-is. Disambiguate any X character, regardless of place.

2. Imitate the legacy behavior exactly. Disambiguate any XX string regardless of place, or use the third argument if the year component contains any uncertainty.

3. Do something new and conservative. Perhaps require any uncertain digits to be followed by only uncertain digits in less significant places:

   • 2000-01-XX – valid
   • 2000-XX-XX – valid
   • 2000-XX-01 – not valid—constrained digit in a less significant place than an ambiguous digit (assuming %Y-%m-%d)
   • 2000-01-0X – could be valid

[tsibley]

@elebow Oh, weird. Nice digging! I would prefer (3) and would consider it a kind of bug fix, but (2) might be more called for so that we don't break compat.

[rneher]

Thanks for this! I agree with @tsibley that behavior 3 is preferable.

[elebow]

Added a check that raises a ValueError when a constrained digit appears in a less-significant place than an uncertain digit. (This is the same error class raised for malformed dates like 2000-02-02-02)

Also rebased on latest master.

Note that this still includes the cherry-picked commit from #527.

[rneher]

this looks good to me. @tsibley, do you have additional comments?

[tsibley]

LGTM!