-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add phylogenetic directory #18
Conversation
Move phylogenetic workflow from top-level to phylogenetic directory to follow the [Pathogen Repo Guide](https://github.com/nextstrain/pathogen-repo-guide/tree/main)
Part of work to update this repo to match the pathogen-repo-guide.
Part of work to update this repo to match the pathogen-repo-guide.
Part of work to update this repo to match the pathogen-repo-guide.
Part of work to update this repo to match the pathogen-repo-guide.
Following the pathogen-repo-guide and nextstrain/zika@efe11e3
Update top-level and phylogenetic `README.md` files to match new workflow structure that includes ingest and phylogenetic directories, following the pathogen-repo-guide
Add empty top-level `nextstrain-pathogen.yaml` to allow `nextstrain build` to work from any directory regardless of runtime, as described [here](nextstrain/pathogen-repo-guide@e318589)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the structured commits & messages - very easy to ready through. I can build the DAG locally so I suspect this is all working as it should, but I'll defer to @joverlee521 who is the best placed to review the changes here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks for reworking it to make clear step-by-step commits!
The CI workflow was able to complete successfully 🎉
In case you haven't explored it yet, you can download the outputs from the CI workflow in the "Artifacts" section. The downloaded outputs should include the artifact_paths
that you defined in the ci.yaml
. This includes the example Auspice JSON output that you can drag on to auspice.us for a quick review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
For future reference, the docstrings in the snakemake rules were reduced in 1b5c544 after discussion with @kimandrews, @j23414, and myself.
As @jameshadfield commented in a separate PR, these docstrings are prone to falling out of sync. We will be stringent about these docstrings in the zika repo because the zika repo will be used in our tutorials, but other pathogen repos can have generalized docstrings to reduce maintenance.
Description of proposed changes
Add phylogenetic directory to follow the pathogen-repo-guide, and update the CI workflow to match the new file structure.
Many changes follow those made in the dengue repo and the zika repo
Checklist