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: Adding AMRFinderPlusAnnotation type #86

Merged

Conversation

VinzentRisch
Copy link
Collaborator

@VinzentRisch VinzentRisch commented Jul 4, 2024

solves #85

  • Added new type SampleData[AMRFinderPlusAnnotations] to be used with SampleData[MAGs] annotation.
  • Added new type FeatureData[AMRFinderPlusAnnotation] to be used with FeatureData[Sequence] annotation.

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 tqdm jellyfish 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/86/head:pr-86
git checkout pr-86
pip install -e .

Download test data:
PR-86.zip

  1. Test it out!
qiime tools import --input-path sample_data --output-path amr_annotations.qza --type "SampleData[AMRFinderPlusAnnotations]"
qiime tools import --input-path feature_data --output-path amr_annotation.qza --type "FeatureData[AMRFinderPlusAnnotation]"

@VinzentRisch VinzentRisch requested a review from a team July 11, 2024 14:55
@VinzentRisch VinzentRisch requested review from ChristosMatzoros and removed request for a team July 11, 2024 14:55
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.

The code looks great and everything works as expected!!

@VinzentRisch
Copy link
Collaborator Author

Hi @misialq
Can you merge this one for me too?
And can you remove those checks that are not needed anymore so i can merge the PRs by myself? The checks are still there also on newly created PRs.

@misialq
Copy link
Contributor

misialq commented Jul 11, 2024

Hey @VinzentRisch, I finally figured why all these check were always appearing on all of your PRs and fixed it - from now on, only the two should be visible. What I am wondering, though, is why the coverage report does not show up here (or anywhere else?) - can you please investigate? I'm merging this one in the meantime.

@misialq misialq merged commit 0b5439a into bokulich-lab:main Jul 11, 2024
2 checks passed
@misialq
Copy link
Contributor

misialq commented Jul 11, 2024

Oh, and let's stop using the `` ticks in commit names/PR titles - it breaks our CI 😅

@VinzentRisch
Copy link
Collaborator Author

On the topic of CI bugs. @misialq
I think revert commits also break the CI
see here: https://github.com/bokulich-lab/q2-amr/actions/runs/9807765273/job/27082173410?pr=86#step:4:12
Or is it something else?

@misialq
Copy link
Contributor

misialq commented Jul 12, 2024

Yeah, it seems so... my suspicion is that the additional quotation marks " in the revert commit message may have something to do with that...

VinzentRisch added a commit to VinzentRisch/q2-amr that referenced this pull request Sep 27, 2024
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.

3 participants