Skip to content

Commit e674856

Browse files
committed
Handle ambiguity in --min-date and --max-date
This fixes 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. numeric_date() and get_numerical_date_from_value() are two separate functions that serve similar purposes: converting some date value to a numeric date. The latter has had more recent developments, and crucially will return a date range for ambiguous dates. This is desired for min/max date filters, so I've switched to it.
1 parent c3476ea commit e674856

4 files changed

Lines changed: 47 additions & 15 deletions

File tree

augur/dates/__init__.py

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import re
77
from functools import cache
88
from treetime.utils import numeric_date as tt_numeric_date, datetime_from_numeric
9-
from typing import Any, Dict, Optional, Tuple, Union
9+
from typing import Any, Dict, Literal, Optional, Tuple, Union
1010
from augur.errors import AugurError
1111
from .errors import InvalidDate
1212

@@ -75,17 +75,49 @@ def numeric_date(date):
7575

7676
raise InvalidDate(date, f"""Ensure it is in one of the supported formats:\n{SUPPORTED_DATE_HELP_TEXT}""")
7777

78-
def numeric_date_type(date):
79-
"""Wraps numeric_date() for argparse usage.
78+
def numeric_date_type_min(date) -> float:
79+
"""Wraps numeric_date() for argparse usage, taking the minimum value if resolved to a range.
8080
8181
This raises an ArgumentTypeError from InvalidDateFormat exceptions, otherwise the custom exception message won't be shown in console output due to:
8282
https://github.com/python/cpython/blob/5c4d1f6e0e192653560ae2941a6677fbf4fbd1f2/Lib/argparse.py#L2503-L2513
83+
84+
>>> round(numeric_date_type_min("2018"), 3)
85+
2018.001
8386
"""
8487
try:
85-
return numeric_date(date)
88+
return get_single_numeric_date(date, fmt="%Y-%m-%d", min_or_max="min")
8689
except InvalidDate as error:
8790
raise argparse.ArgumentTypeError(str(error)) from error
8891

92+
def numeric_date_type_max(date) -> float:
93+
"""Wraps numeric_date() for argparse usage, taking the maximum value if resolved to a range.
94+
95+
This raises an ArgumentTypeError from InvalidDateFormat exceptions, otherwise the custom exception message won't be shown in console output due to:
96+
https://github.com/python/cpython/blob/5c4d1f6e0e192653560ae2941a6677fbf4fbd1f2/Lib/argparse.py#L2503-L2513
97+
98+
>>> round(numeric_date_type_max("2018"), 3)
99+
2018.999
100+
"""
101+
try:
102+
return get_single_numeric_date(date, fmt="%Y-%m-%d", min_or_max="max")
103+
except InvalidDate as error:
104+
raise argparse.ArgumentTypeError(str(error)) from error
105+
106+
def get_single_numeric_date(value, fmt, min_or_max: Literal["min", "max"]) -> float:
107+
numeric_date = get_numerical_date_from_value(value, fmt)
108+
109+
if isinstance(numeric_date, float):
110+
return numeric_date
111+
112+
if isinstance(numeric_date, tuple):
113+
if min_or_max == "min":
114+
return numeric_date[0]
115+
if min_or_max == "max":
116+
return numeric_date[1]
117+
118+
raise InvalidDate(value, f"""Ensure it is in one of the supported formats:\n{SUPPORTED_DATE_HELP_TEXT}""")
119+
120+
89121
def is_date_ambiguous(date, ambiguous_by):
90122
"""
91123
Returns whether a given date string in the format of YYYY-MM-DD is ambiguous by a given part of the date (e.g., day, month, year, or any parts).

augur/filter/__init__.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
program vcftools must be available on PATH.
1313
"""
1414
from augur.argparse_ import ExtendOverwriteDefault, SKIP_AUTO_DEFAULT_IN_HELP
15-
from augur.dates import numeric_date_type
15+
from augur.dates import numeric_date_type_min, numeric_date_type_max
1616
from augur.filter.arguments import descriptions
1717
from augur.filter.io import column_type_pair
1818
from augur.io.metadata import DEFAULT_DELIMITERS, DEFAULT_ID_COLUMNS
@@ -39,8 +39,8 @@ def register_arguments(parser):
3939

4040
metadata_filter_group.add_argument('--query', help=descriptions['query'])
4141
metadata_filter_group.add_argument('--query-columns', type=column_type_pair, nargs="+", action=ExtendOverwriteDefault, help=descriptions['query_columns'])
42-
metadata_filter_group.add_argument('--min-date', type=numeric_date_type, help=descriptions['min_date'])
43-
metadata_filter_group.add_argument('--max-date', type=numeric_date_type, help=descriptions['max_date'])
42+
metadata_filter_group.add_argument('--min-date', type=numeric_date_type_min, help=descriptions['min_date'])
43+
metadata_filter_group.add_argument('--max-date', type=numeric_date_type_max, help=descriptions['max_date'])
4444
metadata_filter_group.add_argument('--exclude-ambiguous-dates-by', choices=['any', 'day', 'month', 'year'], help=descriptions['exclude_ambiguous_dates_by'])
4545
metadata_filter_group.add_argument('--exclude', type=str, nargs="+", action=ExtendOverwriteDefault, help=descriptions['exclude'])
4646
metadata_filter_group.add_argument('--exclude-where', nargs='+', action=ExtendOverwriteDefault, help=descriptions['exclude_where'])

augur/frequencies.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from .errors import AugurError
1111
from .frequency_estimators import get_pivots, alignment_frequencies, tree_frequencies
1212
from .frequency_estimators import AlignmentKdeFrequencies, TreeKdeFrequencies, TreeKdeFrequenciesError
13-
from .dates import numeric_date_type, SUPPORTED_DATE_HELP_TEXT, get_numerical_dates
13+
from .dates import numeric_date_type_min, numeric_date_type_max, SUPPORTED_DATE_HELP_TEXT, get_numerical_dates
1414
from .io.file import open_file
1515
from .io.metadata import DEFAULT_DELIMITERS, DEFAULT_ID_COLUMNS, METADATA_DATE_COLUMN, InvalidDelimiter, Metadata, read_metadata
1616
from .utils import write_json
@@ -37,9 +37,9 @@ def register_parser(parent_subparsers):
3737
help="number of units between pivots")
3838
parser.add_argument("--pivot-interval-units", type=str, default="months", choices=['months', 'weeks'],
3939
help="space pivots by months (default) or by weeks")
40-
parser.add_argument('--min-date', type=numeric_date_type,
40+
parser.add_argument('--min-date', type=numeric_date_type_min,
4141
help=f"date to begin frequencies calculations; may be specified as: {SUPPORTED_DATE_HELP_TEXT}")
42-
parser.add_argument('--max-date', type=numeric_date_type,
42+
parser.add_argument('--max-date', type=numeric_date_type_max,
4343
help=f"date to end frequencies calculations; may be specified as: {SUPPORTED_DATE_HELP_TEXT}")
4444

4545
# Tree-specific arguments

tests/functional/filter/cram/filter-max-date.t

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,16 @@ Create metadata TSV file for testing.
66

77
$ cat >metadata.tsv <<~~
88
> strain date
9-
> SEQ_1 2020-03-XX
10-
> SEQ_2 2020-03-01
11-
> SEQ_3 2020-03-02
9+
> SEQ_1 2019-XX-XX
10+
> SEQ_2 2019-12-31
11+
> SEQ_3 2020-01-01
1212
> ~~
1313

14-
Test that --max-date is inclusive.
14+
Test that --max-date is inclusive even with ambiguity.
1515

1616
$ ${AUGUR} filter \
1717
> --metadata metadata.tsv \
18-
> --max-date 2020-03-01 \
18+
> --max-date 2019 \
1919
> --output-strains filtered_strains.txt 2>/dev/null
2020
$ sort filtered_strains.txt
2121
SEQ_1

0 commit comments

Comments
 (0)