From 731d35d2838ceb57957a8238ec20e65d133f8c5d Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 13 Jan 2025 14:06:16 -0800 Subject: [PATCH 1/3] schema: Make "none" invalid for branches/node_attrs/colorings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ² https://github.com/nextstrain/auspice/pull/1618 --- augur/data/schema-auspice-config-v2.json | 5 +++-- augur/data/schema-export-v2.json | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/augur/data/schema-auspice-config-v2.json b/augur/data/schema-auspice-config-v2.json index ddacab722..fd3707cd2 100644 --- a/augur/data/schema-auspice-config-v2.json +++ b/augur/data/schema-auspice-config-v2.json @@ -22,7 +22,8 @@ "properties": { "key": { "description": "They key used to access the value of this coloring on each node", - "type": "string" + "type": "string", + "not": {"const": "none"} }, "title": { "description": "Text to be displayed in the \"color by\" dropdown and legends", @@ -265,7 +266,7 @@ "$comment": "These columns will not be used as coloring options in Auspice but will be visible in the tree.", "type": "array", "uniqueItems": true, - "items": {"type": "string"} + "items": {"type": "string", "not": {"const": "none"}} }, "extensions": { "description": "Data to be passed through to the the resulting dataset JSON", diff --git a/augur/data/schema-export-v2.json b/augur/data/schema-export-v2.json index 6f8d82f67..86280bd3f 100644 --- a/augur/data/schema-export-v2.json +++ b/augur/data/schema-export-v2.json @@ -281,7 +281,8 @@ "type": "number" } } - } + }, + "^none$": false } }, "branch_attrs": { @@ -296,7 +297,8 @@ "$comment": "e.g. clade->3c3a", "$comment": "string is parsed unchanged by Auspice", "type": "string" - } + }, + "^none$": false } }, "mutations": { From 2a9a141da73b147c884e5b7ca064c5620b9e8b95 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 13 Jan 2025 15:06:51 -0800 Subject: [PATCH 2/3] export_v2: Make "none" an invalid metadata column and coloring key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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.¹ ¹ --- augur/export_v2.py | 20 ++- .../cram/metadata-with-none-column.t | 39 ++++++ .../data/dataset-without-none-column.json | 114 ++++++++++++++++++ .../export_v2/data/none_column_metadata.tsv | 4 + 4 files changed, 174 insertions(+), 3 deletions(-) create mode 100644 tests/functional/export_v2/cram/metadata-with-none-column.t create mode 100644 tests/functional/export_v2/data/dataset-without-none-column.json create mode 100644 tests/functional/export_v2/data/none_column_metadata.tsv diff --git a/augur/export_v2.py b/augur/export_v2.py index a78ca3782..6484eca7d 100644 --- a/augur/export_v2.py +++ b/augur/export_v2.py @@ -25,6 +25,9 @@ MINIFY_THRESHOLD_MB = 5 +# Invalid metadata columns because they are used internally by Auspice +INVALID_METADATA_COLUMNS = ("none") + # Set up warnings & exceptions warn = warnings.warn @@ -401,6 +404,10 @@ def _is_valid(coloring): if key != "gt" and not trait_values: warn(f"Requested color-by field {key!r} does not exist and will not be used as a coloring or exported.") return False + if key in INVALID_METADATA_COLUMNS: + warn(f"You asked for a color-by for trait {key!r}, but this is an invalid coloring key.\n" + "It will be ignored during export, please rename field if you would like to use it as a coloring.") + return False return True def _get_colorings(): @@ -950,10 +957,13 @@ def register_parser(parent_subparsers): config.add_argument('--description', metavar="description.md", help="Markdown file with description of build and/or acknowledgements to be displayed by Auspice") config.add_argument('--warning', metavar="text or file", help="Text or file in Markdown format to be displayed as a warning banner by Auspice") config.add_argument('--geo-resolutions', metavar="trait", nargs='+', action=ExtendOverwriteDefault, help="Geographic traits to be displayed on map") - config.add_argument('--color-by-metadata', metavar="trait", nargs='+', action=ExtendOverwriteDefault, help="Metadata columns to include as coloring options") + config.add_argument('--color-by-metadata', metavar="trait", nargs='+', action=ExtendOverwriteDefault, + help="Metadata columns to include as coloring options. " + + f"Ignores columns named {INVALID_METADATA_COLUMNS!r}, so please rename them if you would like to include them as colorings.") config.add_argument('--metadata-columns', nargs="+", action=ExtendOverwriteDefault, - help="Metadata columns to export in addition to columns provided by --color-by-metadata or colorings in the Auspice configuration file. " + - "These columns will not be used as coloring options in Auspice but will be visible in the tree.") + help="Metadata columns to export in addition to columns provided by --color-by-metadata or colorings in the Auspice configuration file. " + + "These columns will not be used as coloring options in Auspice but will be visible in the tree. " + + f"Ignores columns named {INVALID_METADATA_COLUMNS!r}, so please rename them if you would like to include them as metadata fields.") config.add_argument('--panels', metavar="panels", nargs='+', action=ExtendOverwriteDefault, choices=['tree', 'map', 'entropy', 'frequencies', 'measurements'], help="Restrict panel display in auspice. Options are %(choices)s. Ignore this option to display all available panels.") optional_inputs = parser.add_argument_group( @@ -1195,6 +1205,10 @@ def get_additional_metadata_columns(config, command_line_metadata_columns, metad additional_metadata_columns = [] for col in potential_metadata_columns: + if col in INVALID_METADATA_COLUMNS: + warn(f"You asked for a metadata field {col!r}, but this is an invalid field.\n" + "It will be ignored during export, please rename field if you would like to include as a metadata field.") + continue # Match the column names corrected within parse_node_data_and_metadata corrected_col = update_deprecated_names(col) if corrected_col not in metadata_names: diff --git a/tests/functional/export_v2/cram/metadata-with-none-column.t b/tests/functional/export_v2/cram/metadata-with-none-column.t new file mode 100644 index 000000000..2fa5b2da3 --- /dev/null +++ b/tests/functional/export_v2/cram/metadata-with-none-column.t @@ -0,0 +1,39 @@ +Setup + + $ source "$TESTDIR"/_setup.sh + +Run export with metadata that contains "none" column and asked to use as coloring. +This is expected to output a warning that "none" is an invalid coloring column and skip it in colorings. + + $ ${AUGUR} export v2 \ + > --tree "$TESTDIR/../data/tree.nwk" \ + > --node-data "$TESTDIR/../data/div_node-data.json" "$TESTDIR/../data/location_node-data.json" \ + > --metadata "$TESTDIR/../data/none_column_metadata.tsv" \ + > --color-by-metadata "none" \ + > --maintainers "Nextstrain Team" \ + > --output dataset1.json >/dev/null + WARNING: You asked for a color-by for trait 'none', but this is an invalid coloring key. + It will be ignored during export, please rename field if you would like to use it as a coloring. + \s{0} (re) + + $ python3 "$TESTDIR/../../../../scripts/diff_jsons.py" "$TESTDIR/../data/dataset-without-none-column.json" dataset1.json \ + > --exclude-paths "root['meta']['updated']" "root['meta']['maintainers']" + {} + +Run export with metadata that contains "none" column and asked to use as metadata. +This is expected to output a warning that "none" is an invalid node_attr and skip it in metadata. + + $ ${AUGUR} export v2 \ + > --tree "$TESTDIR/../data/tree.nwk" \ + > --node-data "$TESTDIR/../data/div_node-data.json" "$TESTDIR/../data/location_node-data.json" \ + > --metadata "$TESTDIR/../data/none_column_metadata.tsv" \ + > --metadata-column "none" \ + > --maintainers "Nextstrain Team" \ + > --output dataset2.json >/dev/null + WARNING: You asked for a metadata field 'none', but this is an invalid field. + It will be ignored during export, please rename field if you would like to include as a metadata field. + \s{0} (re) + + $ python3 "$TESTDIR/../../../../scripts/diff_jsons.py" "$TESTDIR/../data/dataset-without-none-column.json" dataset2.json \ + > --exclude-paths "root['meta']['updated']" "root['meta']['maintainers']" + {} diff --git a/tests/functional/export_v2/data/dataset-without-none-column.json b/tests/functional/export_v2/data/dataset-without-none-column.json new file mode 100644 index 000000000..0b090ae5e --- /dev/null +++ b/tests/functional/export_v2/data/dataset-without-none-column.json @@ -0,0 +1,114 @@ +{ + "version": "v2", + "meta": { + "updated": "2025-01-13", + "maintainers": [ + { + "name": "Nextstrain Team" + } + ], + "colorings": [ + { + "key": "location", + "title": "location", + "type": "categorical" + } + ], + "filters": [ + "location" + ], + "panels": [ + "tree" + ] + }, + "tree": { + "name": "ROOT", + "node_attrs": { + "div": 0 + }, + "branch_attrs": {}, + "children": [ + { + "name": "tipA", + "node_attrs": { + "div": 1, + "location": { + "value": "delta" + } + }, + "branch_attrs": {} + }, + { + "name": "internalBC", + "node_attrs": { + "div": 2 + }, + "branch_attrs": {}, + "children": [ + { + "name": "tipB", + "node_attrs": { + "div": 3, + "location": { + "value": "gamma" + } + }, + "branch_attrs": {} + }, + { + "name": "tipC", + "node_attrs": { + "div": 3, + "location": { + "value": "gamma" + } + }, + "branch_attrs": {} + } + ] + }, + { + "name": "internalDEF", + "node_attrs": { + "div": 5, + "location": { + "value": "alpha" + } + }, + "branch_attrs": {}, + "children": [ + { + "name": "tipD", + "node_attrs": { + "div": 8, + "location": { + "value": "alpha" + } + }, + "branch_attrs": {} + }, + { + "name": "tipE", + "node_attrs": { + "div": 9, + "location": { + "value": "alpha" + } + }, + "branch_attrs": {} + }, + { + "name": "tipF", + "node_attrs": { + "div": 6, + "location": { + "value": "beta" + } + }, + "branch_attrs": {} + } + ] + } + ] + } +} \ No newline at end of file diff --git a/tests/functional/export_v2/data/none_column_metadata.tsv b/tests/functional/export_v2/data/none_column_metadata.tsv new file mode 100644 index 000000000..355575762 --- /dev/null +++ b/tests/functional/export_v2/data/none_column_metadata.tsv @@ -0,0 +1,4 @@ +name none +tipA true +tipB true +tipC true From 54cc39f4e972226968267ad525cb944674a44221 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 13 Jan 2025 15:36:16 -0800 Subject: [PATCH 3/3] Update changelog --- CHANGES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index e02ae967e..b4977862b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,12 @@ ## __NEXT__ +### Major Changes + +* export v2: The string "none" is now an invalid value for `--color-by-metadata` and `--metadata-columns` options and will be ignored 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) + +[#1113]: https://github.com/nextstrain/augur/pull/1113 ## 27.2.0 (22 January 2025)