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

ENH: Added new action that can download AMRFinderPlus database called fetch_amrfinderplus_db #82

Merged

Conversation

VinzentRisch
Copy link
Collaborator

@VinzentRisch VinzentRisch commented Jul 2, 2024

solves #77

  • Added new action that downloads the latest version of the AMRFinderPlus database and outputs it in an artifact of type AMRFinderPlusDatabase.

Set up an environment

CONDA_SUBDIR=osx-64 mamba create -yn q2-amr \
  -c https://packages.qiime2.org/qiime2/2024.2/shotgun/released/ \
  -c qiime2 -c conda-forge -c bioconda -c defaults \
  qiime2 q2cli q2templates q2-types rgi ncbi-amrfinderplus

conda activate q2-amr
conda config --env --set subdir osx-64

pip install --no-deps --force-reinstall \
  git+https://github.com/misialq/rgi.git@py38-fix \
  git+https://github.com/bokulich-lab/q2-amr.git

Run it locally

  1. First, clone the repo and checkout the PR branch:
git clone https://github.com/bokulich-lab/q2-amr.git
cd q2-amr
git fetch origin pull/82/head:pr-82
git checkout pr-82
pip install -e .
  1. Test it out!
 qiime amr fetch-amrfinderplus-db --o-amrfinderplus-db amrfinderplus_db.qza 

@VinzentRisch
Copy link
Collaborator Author

Hey can you have a look at this?
Only look at the new action and the tests that go with it, the new formats belong to another PR.

Copy link
Contributor

@ChristosMatzoros ChristosMatzoros left a comment

Choose a reason for hiding this comment

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

It runs as expected and the tests pass. Check some comments that I made.

q2_amr/amrfinderplus/database.py Show resolved Hide resolved
q2_amr/amrfinderplus/database.py Outdated Show resolved Hide resolved
q2_amr/amrfinderplus/database.py Show resolved Hide resolved
Copy link
Contributor

@ChristosMatzoros ChristosMatzoros left a comment

Choose a reason for hiding this comment

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

Seems good to me!

@VinzentRisch VinzentRisch force-pushed the 77_amrfinder_ncbi_database branch from 95242a3 to 1f252ac Compare July 5, 2024 11:41
@VinzentRisch
Copy link
Collaborator Author

Hi
I just changed the way _copy_all copies the files. Now it does not copy the files that are not needed for the database.

Copy link
Contributor

@ChristosMatzoros ChristosMatzoros left a comment

Choose a reason for hiding this comment

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

Seems good to me @VinzentRisch . Just a minor change in the comments.

q2_amr/amrfinderplus/database.py Outdated Show resolved Hide resolved
@VinzentRisch VinzentRisch force-pushed the 77_amrfinder_ncbi_database branch from 8307707 to c12d56c Compare July 16, 2024 14:25
@VinzentRisch
Copy link
Collaborator Author

Hi @ChristosMatzoros I changed the function copy_all slightly with a different regex. Could you run the function again to see if all works well?

Copy link
Contributor

@ChristosMatzoros ChristosMatzoros left a comment

Choose a reason for hiding this comment

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

Works fine!

@VinzentRisch VinzentRisch merged commit 38e48b5 into bokulich-lab:main Jul 16, 2024
2 checks passed
VinzentRisch added a commit to VinzentRisch/q2-rgi that referenced this pull request Sep 27, 2024
…e called `fetch_amrfinderplus_db` (bokulich-lab#82)"

This reverts commit 38e48b5.
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.

2 participants