Generalize Ingest

[j23414]

Description of proposed changes

Ingest data from genbank to generate:

• data/sequences_all.fasta
• data/metadata_all.tsv

for a dengue build.

Instead of separately pulling denv1 to denv4, all types are combined in one file with an annotated column: (2023-03-25, to avoid confusion keep serotypes separate)

• data/sequences_denv1.fasta
• data/metadata_denv1.tsv
• data/sequences_denv2.fasta
• data/metadata_denv2.tsv
• data/sequences_denv3.fasta
• data/metadata_denv3.tsv
• data/sequences_denv4.fasta
• data/metadata_denv4.tsv

Unordered list of remaining tasks that may change:

• Update docs
• Pull and merge cached datasets to avoid recompute
• Pull title from PubMed instead of Description line

Related issue(s)

Testing

• Checks pass

Local Test

Can test this locally by running

git clone https://github.com/nextstrain/dengue.git
cd dengue
git checkout new_ingest
cd ingest
nextstrain build .

ls -ltr data
wc -l data/metadata_all.tsv

May need to install tidyverse (No longer need R since the script was refactored into python)

R
install.packages(tidyverse)

[tsibley]

I haven't done a detailed review, but a couple high-level comments to start off:

1. I don't think just-in-time downloading of programs from monkeypox is the way we want to reuse stuff. It seems to me that it will be fragile and hard to maintain over time.

2. While I personally enjoy multi-lingual projects and am sympathetic to the best tools being the ones you know (and do like tidyverse), I think we should replace the lone R program here with something in Python for consistency with the rest of the repo/ecosystem.

   The R program also won't work in our Docker runtime (e.g. try it with nextstrain build --docker ingest/) and if it works in our Conda and ambient runtimes for someone, it's only by chance. We could add R support to our runtimes, but I think that's a bigger scope of work. I also noted that the R program loads the whole metadata file into memory when it could instead do a streaming transform (e.g. as csv-to-ndjson does).

[huddlej]

This is great, @j23414! It's a big lift to convert tidyverse logic to pandas logic especially since pandas has a huge learning curve and remains confusing in many ways even to people who have used it for years. Most of the comments below touch on Python conventions used throughout the other Nextstrain Python codebase, but there is an important note about the mapping of NCBI lineage ids to serotype names that's more of a data formatting consideration.

[j23414]

I haven't done a detailed review, but a couple high-level comments to start off:

1. I don't think just-in-time downloading of programs from monkeypox is the way we want to reuse stuff. It seems to me that it will be fragile and hard to maintain over time.
2. While I personally enjoy multi-lingual projects and am sympathetic to the best tools being the ones you know (and do like tidyverse), I think we should replace the lone R program here with something in Python for consistency with the rest of the repo/ecosystem.
   The R program also won't work in our Docker runtime (e.g. try it with nextstrain build --docker ingest/) and if it works in our Conda and ambient runtimes for someone, it's only by chance. We could add R support to our runtimes, but I think that's a bigger scope of work. I also noted that the R program loads the whole metadata file into memory when it could instead do a streaming transform (e.g. as csv-to-ndjson does).

Point 2 is addressed with 2c8553a

Re: Point 1, I still disagree...mostly because I'm currently trying to propagate changes across zika and ebola ingests which is already tedious when copying changes across their snakefiles. One might argue to only polish ingest scripts on one repo (dengue), however in my experience that results in over-specialized scripts that become really difficult to generalize later.

To meet the final end-goal of Point 1 (if not the immediate developmental path of Point 1), I'm happy to make final copies in a future PR after I'm sure the pipeline works for:

• nextstrain/zika@main...ingest
• nextstrain/ebola@master...ingest

Otherwise my dev path has already branched into:

• https://github.com/nextstrain/dengue/tree/new_ingest_uniqmerge - to ensure cached information is not lost in a new ingest
• https://github.com/nextstrain/dengue/tree/new_ingest_build - to add the modified build rules similar to nextstrain/zika@d9600f7

[tsibley]

@j23414 Nod. I don't want to get in the way of your active development.

As a final state, though, I still think we should not rely on this sort of dynamic downloading. It's got all the problems of a package management system (dependencies, versioning, updates, etc), without any of the solutions a mature package management system provides.

There's also issues of brittle close coupling it introduces. Consider that someone changing files in the monkeypox workflow would (rightfully so, I'd argue) not think that modifying one of the programs in scripts/ could break the unrelated dengue workflow.

[tsibley]

I normally review PRs commit-by-commit, but this was not feasible here given the commit structure and sequence. Instead, I reviewed this PR as one large diff after excluding the initial verbatim copy of the ingest machinery from monkeypox ("add ingest from monkeypox repo" (645acb2)).

# HEAD was "fix: switch to augur curate normalize-strings" (b41fb45)
git diff --compact-summary --patch 645acb2..@

This exclusion helped reduce the amount under review, since it's been reviewed previously, and highlight what had to change from monkeypox to here.

A couple general top-level comments, before specifics:

• It'd be good to incorporate changes from Parameterize ncbi_id in fetch_sequences mpox#146 here, one way or another. I'm sure that's your plan! but calling it out so we don't forget.

• Running nextstrain build ingest produces files in ingest/.snakemake/ and ingest/logs/ that should be in a git ignore file.

[j23414]

Working on rebasing this! Do not review yet! Thanks @victorlin!!!

[j23414]

This PR superseded by merged PRs:

• Copy ingest #13
• Harmonize ingest with pathogen repo guide #35
• Automate ingest and phylogenetic workflows #38