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

export v2 / validate auspice-config-v2 fails with "display_defaults.branch_label" element #444

Closed
brianpardy opened this issue Feb 18, 2020 · 3 comments

Comments

@brianpardy
Copy link

I attempted to specify the "display_defaults.branch_label" attribute in my auspice_config.json file as per the documentation at https://nextstrain.github.io/auspice/advanced-functionality/view-settings and received an error from "augur export v2":

        ERROR: Additional properties are not allowed ('branch_label' was unexpected). Trace: properties - display_defaults - additionalProperties
Validation of config/auspice_config.json failed. Please check the formatting of this file & refer to the augur documentation for further help. 
Validating produced JSON
Validating produced JSON
[Mon Feb 17 21:59:06 2020]
Error in rule export:
    jobid: 5
    output: results/ncov_with_accessions.json
    shell:
        
        augur export v2             --tree results/tree.nwk             --metadata data/metadata.tsv             --node-data results/branch_lengths.json results/nt_muts.json results/aa_muts.json results/traits.json             --auspice-config config/auspice_config.json             --colors config/colors.tsv             --lat-longs config/lat_longs.tsv             --description config/description.md             --output results/ncov_with_accessions.json
        
        (one of the commands exited with non-zero exit code; note that snakemake uses bash strict mode!)

This also replicates by running simply "augur validate auspice-config-v2 config/auspice_config.json" without the call to export.

hack% augur validate auspice-config-v2 config/auspice_config.json
        ERROR: Additional properties are not allowed ('branch_label' was unexpected). Trace: properties - display_defaults - additionalProperties
FATAL ERROR: Validation failed.

The augur/data/schema-auspice-config-v2.json contains no reference to the branch_label attribute as permitted in the schema. If I edit this file and add a definition for branch_label, type string, enum [ "aa", "none" ], the implicit validation step in "augur export v2" completes without an error:

hack% augur validate auspice-config-v2 config/auspice_config.json
Validation succeeded

However, export v2 still strips the display_defaults.branch_label attribute from the resulting exported .json file. I tried editing augur/data/schema-export-v2.json to see if that was sufficient to allow export v2 to pass the display_defaults.branch_label element to the export .json but that did not help with my issue.

Editing augur/export_v2.py to add branch_label to the "v1_v2_keys" array appears to allow export v2 to pass the display_defaults.branch_label element to the exported .json and accomplish the behavior I intended, which was to default to including aa branch_labels in auspice. I applied these changes to my fork of augur and can submit a pull request, but I am not sure this is the best fix.

@emmahodcroft
Copy link
Member

Hi @brianpardy, I'm sorry this is causing you problems. You've caught us out here!

We fast-tracked an implementation of a way to specify the branch_labels to display by default by auspice to help with the current SARS-CoV-2 runs & narratives.
However, this isn't yet integrated into augur :)

For the moment, the easiest way to handle this at the moment is to export your JSON file without branch_label in the config file, and then open up the resulting JSON file in a text-editor and add "branch_label": "aa" to the display_defaults section. This should result in your desired behaviour.

The reason we haven't integrated this into augur is we don't have a way, using the pipeline tools, to have any branch labels other than clade and aa (and clade is always displayed by default if present) and we think this feature will be most useful when we have more options to provide different clade labels in augur that can then be chosen.

However, you raise a valid point that there's no reason specifying aa in the config should cause the validate to fail, so we can work on trying to rectify this! Thanks for letting us know!

@brianpardy
Copy link
Author

Thank you for the explanation, @emmahodcroft! The changes in your pull request match mine and have been working perfectly.

@jameshadfield
Copy link
Member

Hey @brianpardy -- this has been closed by #442 and is in augur release 6.4.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants