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

Remove NCBI Virus #19

Merged
merged 3 commits into from
Sep 14, 2023
Merged

Remove NCBI Virus #19

merged 3 commits into from
Sep 14, 2023

Conversation

joverlee521
Copy link
Contributor

@joverlee521 joverlee521 commented Sep 14, 2023

Resolves #18 by just completely removing the NCBI Virus scripts.

We can just stick to Entrez and NCBI Datasets for fetching public data.

Checklist

  • Checks pass
  • If adding a script, add an entry for it in the README.

Due to #18, the NCBI Virus
API is more of a hassle to use. The data from NCBI Datasets CLI covers
the standard fields that we use in pathogen ingests, so we can drop
the use of the undocumented NCBI Virus API.

If any pathogen needs additional custom fields that are not available
through NCBI Datasets, the pipeline can use fetch-from-ncbi-entrez
and parse the GenBank file.
Trim back CI jobs for reasons stated in
nextstrain/cli@fab709a
The only tests we had were for fetch-from-ncbi-virus, which were removed
in 6a76d0b
Copy link
Member

@victorlin victorlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable given these are now unused in downstream repos, as mentioned in #18 (comment).

@joverlee521 joverlee521 merged commit 9cfed8b into main Sep 14, 2023
@joverlee521 joverlee521 deleted the remove-ncbi-virus branch September 14, 2023 20:03
@tsibley
Copy link
Member

tsibley commented Sep 14, 2023

8 files changed, 8 insertions(+), 480 deletions(-)

🎉 Love a good net-negative change!

joverlee521 added a commit to nextstrain/pathogen-repo-guide that referenced this pull request Sep 14, 2023
Dropping due to the changes of the NCBI Virus API described in
nextstrain/ingest#18 and we are dropping the
scripts for this in nextstrain/ingest#19.
joverlee521 added a commit to nextstrain/avian-flu that referenced this pull request May 21, 2024
Copying two scripts that were in nextstrain/ingest before they were
removed in <nextstrain/ingest#19>

- ncbi-virus-url
- fetch-from-ncbi-virus

Done in preparation for editing the NCBI Virus URL to be used for
ingest because the NCBI Datasets does not include strain, segment,
and serotype fields.
joverlee521 added a commit to nextstrain/avian-flu that referenced this pull request May 24, 2024
Copying two scripts that were in nextstrain/ingest before they were
removed in <nextstrain/ingest#19>

- ncbi-virus-url
- fetch-from-ncbi-virus

Done in preparation for editing the NCBI Virus URL to be used for
ingest because the NCBI Datasets does not include strain, segment,
and serotype fields.
joverlee521 added a commit to nextstrain/avian-flu that referenced this pull request May 29, 2024
Copying two scripts that were in nextstrain/ingest before they were
removed in <nextstrain/ingest#19>

- ncbi-virus-url
- fetch-from-ncbi-virus

Done in preparation for editing the NCBI Virus URL to be used for
ingest because the NCBI Datasets does not include strain, segment,
and serotype fields.
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.

fetch-from-ncbi-virus does not include nucleotide sequences
3 participants