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

Add phylogenetic directory #18

Merged
merged 10 commits into from
Mar 1, 2024
Merged

Add phylogenetic directory #18

merged 10 commits into from
Mar 1, 2024

Conversation

kimandrews
Copy link
Contributor

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

  • Checks pass

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)
@kimandrews kimandrews requested a review from a team February 26, 2024 19:07
Copy link
Member

@jameshadfield jameshadfield left a 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.

Copy link
Contributor

@joverlee521 joverlee521 left a 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.

Copy link
Contributor

@j23414 j23414 left a 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!

@kimandrews kimandrews requested a review from joverlee521 March 1, 2024 18:52
Copy link
Contributor

@joverlee521 joverlee521 left a 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.

@kimandrews kimandrews merged commit 9377ef6 into main Mar 1, 2024
32 checks passed
@kimandrews kimandrews deleted the add-phylogenetic branch March 1, 2024 22:28
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

Successfully merging this pull request may close these issues.

4 participants