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

feat: support treetime ambiguous date format #1305

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions augur/dates/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ def get_numerical_date_from_value(value, fmt=None, min_max_year=None):
except InvalidDate as error:
raise AugurError(str(error)) from error
return [treetime.utils.numeric_date(d) for d in ambig_date]
if value.startswith('[') and value.endswith(']') and len(value[1:-1].split(':'))==2:
Copy link
Member

Choose a reason for hiding this comment

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

For added context, this is the corresponding logic in TreeTime: treetime/utils.py#L305-L312

# Treetime ambiguous date format, e.g. [2019.5:2020.5]
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious how you get the numerical dates in metadata. Is this a standard format outside of TreeTime, or do you run ISO dates through something like treetime.utils.numeric_date to get the float representation?

Asking because I think a good next step after this gets merged is to generalize for any [A:B] date pair where A and B can be anything that's currently supported as an exact scalar date (e.g. [2023-07-31:2023-08-06]). I did some prototyping of this a while back but never got back to creating a proper PR for it.

try:
return [float(x) for x in value[1:-1].split(':')]
except ValueError as error:
raise AugurError(str(error)) from error
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: customize the error message.

When given something like [2023-07-31:2023-08-06], the ValueError message looks something like:

could not convert string to float: '2023-07-31'

It's obvious what this means to us as developers, but maybe not so obvious to users. Something like this might be more clear:

The date range format must be `[A:B]` where both `A` and `B` are numerical dates.

try:
return treetime.utils.numeric_date(datetime.datetime.strptime(value, fmt))
except:
Expand Down
4 changes: 4 additions & 0 deletions tests/dates/test_dates.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,7 @@ def test_get_numerical_dates_dict_error(self):
}
with pytest.raises(AugurError):
dates.get_numerical_dates(metadata)

def test_get_numerical_date_from_value_treetime_ambiguous_date_format(self):
assert dates.get_numerical_date_from_value("[2019.5:2020.5]") == [2019.5, 2020.5]
assert dates.get_numerical_date_from_value("[2013.543:2013.544]") == [2013.543, 2013.544]
Loading