Improve date handling #1519
Draft
Improve date handling #1519
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously a tree was considered temporal (i.e. you could use a temporal tree metric) or not based on the presence of `num_date` on the root node. The allowed partially temporal trees (e.g. those where some tip dates were known) to be displayed as div trees with a colouring of `num_date`, however the date slider wasn't available. Here we allow "partially temporal trees" which are defined as tree(s) where the number of nodes with `num_date` is more than zero but fewer than the total node count. These get a date-range filter, but aren't able to be displayed as true time trees [1]. Note that this has one strange / misleading effect. For tips without a `num_date` they are never filtered out by the date slider. This is obvious when colouring by `num_date` (they are grey) but when using a different colouring one must consider that a node may be visible because it is inside the selected date range _or_ it doesn't have a date! [1] I explored hiding nodes without date information but the results weren't ideal - it could be done, but it would require modifications to the tree layout code.
This implements a feature I've long wanted & it does it the quick and easy way: by requiring that the value already be numeric! In the future it'd be nice to allow YYYY-MM-DD values, but that's for another PR (and comes with its own complexities - what about -XX dates?) This commit will require a corresponding one in augur to update the schema
jameshadfield
force-pushed
the
improve-date-handling
branch
from
b1e0741 to
b5ab0ae
Compare
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously a tree was considered temporal (i.e. you could use a temporal tree metric) or not based on the presence of
num_dateon the root node. The allowed partially temporal trees (e.g. those where some tip dates were known) to be displayed as div trees with a colouring ofnum_date, however the date slider wasn't available. Here's what this looked like:The first commit in this PR allows "partially temporal trees" which are defined as tree(s) where the number of nodes with
num_dateis more than zero but fewer than the total node count. These get a date-range filter, but aren't able to be displayed as true time trees:Note that this has one strange / misleading effect. For tips without a
num_datethey are never filtered out by the date slider. This is obvious when colouring bynum_date(they are grey) but when using a different colouring one must consider that a node may be visible because it is inside the selected date range or it doesn't have a date! For instance here's the MPX tree filtered to dates before 2013 😨The second commit allows temporal scales (
"type": "numeric") as long as they are numeric in the JSON. In the future it'd be nice to allow YYYY-MM-DD values, but that's for another PR (and comes with its own complexities - what about -XX dates?). Here's some randomised data, with 10% of nodes undefined: