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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
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
Expand Up @@ -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.1.0 (15 January 2025)

Expand Down
5 changes: 3 additions & 2 deletions augur/data/schema-auspice-config-v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
6 changes: 4 additions & 2 deletions augur/data/schema-export-v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@
"type": "number"
}
}
}
},
"^none$": false
}
},
"branch_attrs": {
Expand All @@ -292,7 +293,8 @@
"$comment": "e.g. clade->3c3a",
"$comment": "string is parsed unchanged by Auspice",
"type": "string"
}
},
"^none$": false
}
},
"mutations": {
Expand Down
20 changes: 17 additions & 3 deletions augur/export_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -949,10 +956,13 @@ def register_parser(parent_subparsers):
config.add_argument('--build-url', type=str, metavar="url", help="Build URL/repository to be displayed by Auspice")
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('--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(
Expand Down Expand Up @@ -1182,6 +1192,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:
Expand Down
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