Invalid none key #1113
Invalid none key #1113
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1113 +/- ##
==========================================
+ Coverage 72.90% 72.94% +0.04%
==========================================
Files 79 79
Lines 8299 8306 +7
Branches 1694 1696 +2
==========================================
+ Hits 6050 6059 +9
+ Misses 1961 1960 -1
+ Partials 288 287 -1 ☔ View full report in Codecov by Sentry. |
|
Looks good, needs a changelog entry though. |
|
@joverlee521 how would you feel about rebasing this and adding a Changelog entry so it can get merged? Still seems like a worthwhile change, yeah? |
joverlee521
force-pushed
the
invalid-none-key
branch
from
a0756a9 to
50c66e0
Compare
|
Rebased and added changelog entry. I've also made additional changes to make "none" an invalid This is ready for a fresh review after ~2 years 😅 |
joverlee521
force-pushed
the
invalid-none-key
branch
from
50c66e0 to
6ca6dac
Compare
CHANGES.md
Outdated
| * export v2: The string "none" is now an invalid value for `--color-by-metadata` and `--metadata-columns` options to prevent clashes with Auspice's internal use of "none". [#1113][] (@joverlee521) | ||
| * schema: The string "none" is now an invalid branch label, node_attr key, and coloring key. [#1113][] (@joverlee521) |
There was a problem hiding this comment.
Are these technically
breaking
There was a problem hiding this comment.
Ah yes! Thanks for catching that. Will rebase and update changelog after latest release is complete.
There was a problem hiding this comment.
Updated in fd2cff9
Make "none" an invalid key for "tree.branch_attrs.labels", "tree.node_attrs", and "colorings" because the "none" key is used internally in Auspice to hide branch labels¹ and tip labels². ¹ https://github.com/nextstrain/auspice/blob/a4487b59989270b860d2508636f09ebdbc50d0f8/src/util/treeJsonProcessing.js#L56 ² nextstrain/auspice#1618
Update `augur export v2` to warn about the invalid metadata column and coloring "none" key and skip it in export so that it does not clash with Auspice's internal use of "none" to hide tip labels.¹ ¹ <nextstrain/auspice#1618>
joverlee521
force-pushed
the
invalid-none-key
branch
from
6ca6dac to
fd2cff9
Compare
Description of proposed changes
export v2to warn about invalid "none" key and ignore it in exports.export v2for branch labels to ignore the "none" key because it is currently hard-coded to only export "clades" and "aa" as branch labels. We should ignore "none" keys for branch labels once we allow for arbitrary labels in Allow users to specify arbitrary branch & clade labels #728.Related issue(s)
Related to nextstrain/auspice#1618
Testing
Checklist