Allow YYYY-MM-DD values in a temporal scales #1832
Conversation
nextstrain-bot
temporarily deployed
to
auspice-james-temporal--ukfrsq
|
Super cool, @jameshadfield! How do you see the new |
There was a problem hiding this comment.
Nice work! I'm slightly cautious that this might cause confusion for users based on my own experience. Not sure why, but the coloring of sample date in the rabies example really confused me with the divOnly tree. Maybe fixing #1833 will lower the confusion, but I think there needs to be some way to highlight the distinction between the num_date of temporal trees and temporal colorings.
Modifies the utility function to optionally allow ambiguous dates such
as YYYY-MM-XX or YYYY-XX-XX. Additionally error checking has been
improved and the return value for any non-valid string date is now
always `undefined`.
Out of interest I timed the performance of (unambiguous) date parsing
before / after this change and saw only a small change: before:
0.0021ms, after: 0.0023ms (~2 microseconds). Code used for timing:
```js
window.timeit = (allowInvalidStrings = false) => {
const n = 100_000;
const values = Array.from(Array(n)).map(() =>
String(parseInt(String(2000 + Math.random()*20))) + "-" +
String(parseInt(String(Math.random()*12))).padStart(2, '0') + "-" +
String(parseInt(String(Math.random()*28))).padStart(2, '0')
)
const start = performance.now();
values.forEach((v) => calendarToNumeric(v))
console.log(`Time taken ${(performance.now() - start)/n}ms per call`)
}
```
Previously invalid dates would result in an invalid date range, often resulting in no tips selected and errors in the console. The resulting SVG errors due to NaNs may have resulted in the tree not being rendered correctly in some browsers. The error messages were influenced by code review comments on <#1832>
In it's current form it is never used, as the metadata handed to `modifyStateViaMetadata` is created by `createMetadataStateFromJSON` which will never include 'date_range'. Neither is it part of the schema <https://github.com/nextstrain/augur/blob/master/augur/data/schema-export-v2.json> This code was originally added in 2017 <ed745b9> and has been refactored multiple times since then.
into separate functions so that we can more easily make modifications to the temporal one in subsequent commits
These values may be ambiguous (using "XX" notation). There is no ability to provide a custom scale / legend for temporal scales, with errors printed to the console if these are attempted.
Anchor points can be YYYY-MM-DD or numeric dates.
jameshadfield
force-pushed
the
james/temporal-yyyy-mm-dd
branch
from
bbed24b to
6cad339
Compare
|
This PR didn't introduce temporal scales for arbitrary metadata, it simply allowed values to be YYYY-MM-DD strings rather than numbers. Review comments have raised some valid points about the temporal scales themselves:
Good find! I'll make an issue for this. This should work (but I'm not surprised it doesn't) and show a temporal x-axis, but of course there'll be no branches displayed.
Sure - if you can think of visual aids (beyond #1833) we can use them. The tree's x-axis units and title indicate that it's a divergence tree. I guess I don't see it as different to colouring by date but looking at the divergence tree which has always been possible. |
That's what I was expecting, but the available controls are slightly different when there's only a div tree. I was mainly confused why the date range filter was not available. Hiding the control sections is enough for me for now, maybe we can revisit if questions come up from external users. |
|
Oh yeah, that's fair. I actually prototyped a date range filter for this kind of metadata but I wasn't happy with the overall behaviour of the feature. |
Includes a number of improvements to date parsing and validation as well (see commits).
Temporal scales previously could only use numeric dates (2024.123 etc) but can now also use YYYY-MM-DD values including ambiguous values (YYYY-XX-XX, YYYY-MM-XX). For ambiguous values the midpoint of the ambiguous range is used to pick the colour / match the legend swatches to tips.
Example datasets (rabies):
Closes #1427