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

Invalid none key #1113

Merged
merged 3 commits into from
Jan 23, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -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)

5 changes: 3 additions & 2 deletions augur/data/schema-auspice-config-v2.json
Original file line number Diff line number Diff line change
@@ -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",
6 changes: 4 additions & 2 deletions augur/data/schema-export-v2.json
Original file line number Diff line number Diff line change
@@ -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": {
20 changes: 17 additions & 3 deletions augur/export_v2.py
Original file line number Diff line number Diff line change
@@ -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:
39 changes: 39 additions & 0 deletions tests/functional/export_v2/cram/metadata-with-none-column.t
Original file line number Diff line number Diff line change
@@ -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']"
{}
114 changes: 114 additions & 0 deletions tests/functional/export_v2/data/dataset-without-none-column.json
Original file line number Diff line number Diff line change
@@ -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": {}
}
]
}
]
}
}
4 changes: 4 additions & 0 deletions tests/functional/export_v2/data/none_column_metadata.tsv
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name none
tipA true
tipB true
tipC true
Loading