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

Export ambiguous date strings for tips #1760

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jameshadfield
Copy link
Member

See commit messages for details.

This is the num_date case of the more general #386

Auspice PR nextstrain/auspice#1943 will read these exported values, and that PR has a visual summary of what it enables.

This is only exported when using `--timetree` and is intended to be
used by `augur export` to inform Auspice whether a date is inferred or
not. An alternative approach would be for `augur export` to parse the
already exported `raw_date` and decide whether that includes ambiguity
but with the proposed changes in <#1304>
this approach is cleaner and less error-prone.

Note that `raw_date_constraint` is None on internal nodes and a list
(of len 2) on terminal nodes with ambiguous dates.
These provide two important pieces of information for Auspice:

1. A way to unambiguously know if a tip date was inferred or not.
2. When a tip date was inferred we can now display the raw (ambiguous)
   date string.

Note that the new output would validate on the old schema as additional
properties are allowed (by default).
@jameshadfield
Copy link
Member Author

CI is failing because the docs failed to build. Running ./devel/regenerate-developer-api-docs produces no changes and I can run make -C docs html locally without issue.

Copy link

codecov bot commented Feb 18, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 73.21%. Comparing base (23b9fff) to head (bf1c02d).

Files with missing lines Patch % Lines
augur/export_v2.py 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1760      +/-   ##
==========================================
- Coverage   73.23%   73.21%   -0.03%     
==========================================
  Files          79       79              
  Lines        8389     8396       +7     
  Branches     1709     1712       +3     
==========================================
+ Hits         6144     6147       +3     
- Misses       1957     1961       +4     
  Partials      288      288              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jameshadfield
Copy link
Member Author

Failing docs are due to this issue which arises on Sphinx v8.2.0. I'm not sure of the best course of action w.r.t fixing the docs, but their failure is unrelated to the work in this PR and thus shouldn't be blocking.

Comment on lines +865 to +866
node["node_attrs"]["num_date"]["inferred"] = raw_data['date_inferred']
if raw_data['date_inferred'] and raw_data.get('raw_date', False):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the check for raw_data['date_inferred'] in L866 after it's already been used in L865?

Copy link
Member Author

Choose a reason for hiding this comment

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

It can be false (not inferred, i.e. known) and if so then date==raw_date so we don't export the raw date string.

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