Fix phylo input - Githubissues

nextstrain / dengue

Nextstrain build for dengue virus

https://nextstrain.org/dengue

8 stars 10 forks source link

Fix phylo input #40

Closed joverlee521 closed 3 months ago

joverlee521 commented 3 months ago

Description of proposed changes

I missed this in review of https://github.com/nextstrain/dengue/pull/38. The phylogenetic workflow was still pulling from old S3 URLs and not the ingest workflow output data.

Checklist

[x] Checks pass

joverlee521 commented 3 months ago

Triggered trial phylogenetic workflow run with

gh workflow run phylogenetic.yaml --ref fix-phylo-input -f trial_name=fix-phylo-input

j23414 commented 3 months ago

Thank you for working on this! I'll be curious how you solve the problem of adding an "Accession URL". The accession is important for investigating outliers in the tree or getting to the full GenBank entry.

I'd been using a post-processing script in zika: https://github.com/nextstrain/zika/blob/9892009b95ea802f288142734614e29f00212922/phylogenetic/scripts/set_final_strain_name.py#L6-L15

But I think you mentioned there may be a more straightforward method: https://github.com/nextstrain/zika/issues/32

joverlee521 commented 3 months ago

Thank you for working on this! I'll be curious how you solve the problem of adding an "Accession URL". The accession is important for investigating outliers in the tree or getting to the full GenBank entry.

I'd been using a post-processing script in zika: https://github.com/nextstrain/zika/blob/9892009b95ea802f288142734614e29f00212922/phylogenetic/scripts/set_final_strain_name.py#L6-L15

But I think you mentioned there may be a more straightforward method: nextstrain/zika#32

Thanks for bringing this up @j23414, I totally forgot about this issue. I've fixed it with ~8acbbcaee3397b0cea82447b7ad44e68008f3c2f~ 71f52b90fbd99c3ef65a8c97953f5e373ceb3e09. Let me know if you have any questions!

joverlee521 commented 3 months ago

Hmm, CI failing is a separate issue due to installation of the Conda runtime failing.

joverlee521 commented 3 months ago

Trial build has completed and deployed to staging:

https://nextstrain.org/staging/dengue/trial/fix-phylo-input/dengue/all/genome https://nextstrain.org/staging/dengue/trial/fix-phylo-input/dengue/denv1/genome https://nextstrain.org/staging/dengue/trial/fix-phylo-input/dengue/denv2/genome https://nextstrain.org/staging/dengue/trial/fix-phylo-input/dengue/denv3/genome https://nextstrain.org/staging/dengue/trial/fix-phylo-input/dengue/denv4/genome

joverlee521 commented 3 months ago

I'll wait to merge this on Monday so I can monitor for anything unexpected.

I'll probably drop https://github.com/nextstrain/dengue/pull/40/commits/5c850676330dbc2a3c760351ebcd3e989a98a5f7 before merging since I'm working on a better way to split out ingest/phylogenetic GH Action workflows in https://github.com/nextstrain/zika/tree/reusable-workflows

joverlee521 commented 3 months ago

Manually deleted cache and triggered a new run of ingest-to-phylogenetic so that the live builds are actually using the ingested data.