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

BUG: extensions are placed at top level and not under meta as Nextclade expects #885

Closed
corneliusroemer opened this issue Apr 6, 2022 · 2 comments · Fixed by #888
Closed
Labels
bug Something isn't working

Comments

@corneliusroemer
Copy link
Member

#865 allows an extensions JSON object to be defined in auspice_config.json and passed through.

Unfortunately, I didn't realize that the current implementation places extensions at the top level and not under meta as Nextclade expects.

I had thought the discussion on Slack and my issue was clear about this expectation. But unfortunately it was nowhere mentioned that we had settled on meta.extensions.

Given that Nextclade is the only user of this feature so far and we have just released Augur 14.1, would it be possible to make a change from extensions to meta.extensions? @jameshadfield

Maybe we can squeeze this in with the next release? The change would be quite trivial I think.

Alternatively we could accept both - but that would be asking for trouble. Changing what Nextclade expects would be troublesome.

Given that we were undecided about extensions or meta.extensions I feel we should just stick with whatever convention Nextclade has adopted.

I'm sorry this slipped through - I should have checked the PR better 😬

@tsibley
Copy link
Member

tsibley commented Apr 7, 2022

To clarify, extensions remains top-level in the Auspice config schema but would change to be within meta in the dataset schema?

@corneliusroemer
Copy link
Member Author

Yes correct. I think this may also be what one naively expects, since all the other auspice config properties get moved into meta anyways.

Repository owner moved this from New to Done in Nextstrain planning (archived) Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants