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

Changes neccesary for Nextclade update to v.1.10.0 #101

Closed
corneliusroemer opened this issue Jan 25, 2022 · 6 comments
Closed

Changes neccesary for Nextclade update to v.1.10.0 #101

corneliusroemer opened this issue Jan 25, 2022 · 6 comments

Comments

@corneliusroemer
Copy link

I wanted to give you a small heads up. I found your repo through a quick code search, because it doesn't use --input-dataset for Nextclade run. Thus if you update to v1.10.0 (it was released yesterday), you will have to add a line. We recommend to use --input-dataset, but you can also add --input-virus-properties excplicitly.

See this issue for a detailed explanation: nextstrain/nextclade#703

You need to make a change like here: broadinstitute/viral-pipelines@97fd339

Sorry for the trouble.

@corneliusroemer
Copy link
Author

Patch version 1.10.1 comes with a more helpful error message in this use case.

https://github.com/nextstrain/nextclade/releases/tag/1.10.1

Here are notes that should help migrating:

Nextclade CLI 1.10.1 (2022-01-26)

[Fix] Improve error message when the virus properties file is missing #704

Since version 1.10.0 Nextclade CLI have introduced a new required input file, virus_properties.json and datasets and documentation were updated to match. However, users who don't use datasets might have encountered breakage due to a missing file: when running Nextclade CLI without either --input-dataset of --input-virus-properties flag provided, it would stop with an unclear error message. In this release we improve the error message, making sure that that explains the problem and offers a solution.

This does not affect Nextclade Web or Nextalign CLI.

In order to facilitate upgrades, for most users, we recommend to:

  • download the latest dataset before each Nextclade CLI session (e.g. in the beginning of an automated workflow, or once you start a batch of experiments manually) using nextclade dataset get command
  • use --input-dataset flag instead of individual --input-* flags for dataset files when issuing nextclade run command
  • if necessary, override some of the individual input files using corresponding --input-* flags

[Fix] Add information about virus_properties.json or --input-virus-properties to changelog

In the excitement of bringing the new features, we forgot to mention virus_properties.json or --input-virus-properties in the changelog when Nextclade CLI 1.10.0 was released. We now added this information retroactively.

@kapsakcj
Copy link
Contributor

Thanks for the heads up and the information @corneliusroemer , we seriously appreciate it 👍

We're planning on upgrading nextclade version along with changes to how we're invoking it within our workflows.

@kevinlibuit
Copy link
Contributor

Yes, thank you for the heads up, @corneliusroemer! And citing a solution already implemented by @dpark01 is incredibly helpful. We'll likely be making these exact modifications

@dpark01
Copy link
Contributor

dpark01 commented Jan 27, 2022

As mentioned in another slack somewhere, that commit is part of a larger PR here:
broadinstitute/viral-pipelines#404

@corneliusroemer
Copy link
Author

@kevinlibuit As the changelog (now) mentions, it may be more future proof to also add --input-dataset, there's no real downside to it since you can override behaviour if you want to with say --input-virus-properties but if we were to add another file in the future, similar to --input-virus-properties now, you wouldn't have to change anything as long as you download default datasets and have --input-dataset as an arg.

@dpark01
Copy link
Contributor

dpark01 commented Jan 27, 2022

Yeah I'm likely going to be making a change as you just described in viral-pipelines in the WDL task, since the task already does nextclade datasets get anyway, it might as well default to using the whole dataset and still exposing the knobs to override each file.

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

4 participants