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: add action fetch-busco-db and modify evaluate-busco accordingly #162

Merged
merged 72 commits into from
Jun 7, 2024

Conversation

Sann5
Copy link
Contributor

@Sann5 Sann5 commented May 10, 2024

Add the action fetch-busco-db, which downloads a busco database from the internet. Modify the evaluate-busco action to be able to take this database as input. This PR replaces #143 which is too outdated to merge.

Run it locally

  1. Checkout the PR branch.

Assuming 1) you already have a local copy of q2-moshpit installed in an editable mode in your activated virtual environment and 2) the working directory is q2-moshpit; run the following.

PR=162
git fetch origin pull/${PR}/head:pr-${PR}
git checkout pr-${PR}

Also your qiime package should be installed locally and checkout in the second to last commit. E.g.

cd clone_here
git clone https://github.com/qiime2/qiime2.git
git checkout 7b0e435
pip install -e .
  1. Let's get you some data
cd <download here>
wget https://polybox.ethz.ch/index.php/s/slGyW1uQbyxmyb9/download -O mags.qza
  1. Test it out!
# download busco db
qiime moshpit fetch-busco-db \
   --p-virus False --p-prok True --p-euk False \
   --o-busco-db busco_prok_db.qza \
   --verbose
# run busco
qiime moshpit evaluate-busco \
   --i-bins mags.qza \
   --i-busco-db busco_db_prok.qza
   --o-visualization mags.qzv \
   --o-results-table results.qza \
   --p-lineage-dataset bacteria_odb10 \
   --p-num-partitions 3 \
   --verbose \
   --parallel
# check out the visualization
qiime tools view mags.qzv

@Sann5 Sann5 requested review from a team and ChristosMatzoros and removed request for a team May 10, 2024 12:47
@Sann5 Sann5 self-assigned this May 10, 2024
@Sann5
Copy link
Contributor Author

Sann5 commented May 13, 2024

@ChristosMatzoros don't review this yet. We decided to include the code of the dependent PR already in this PR things might look quite different in a bit.

@Sann5 Sann5 changed the title ENH: add action fetch-busco-db ENH: add action fetch-busco-db and modify evaluate-busco accordingly May 14, 2024
Copy link

codecov bot commented May 14, 2024

Codecov Report

Attention: Patch coverage is 96.06299% with 5 lines in your changes missing coverage. Please review.

Project coverage is 96.82%. Comparing base (4d7ccf8) to head (e014115).

Current head e014115 differs from pull request most recent head 5a3b4b2

Please upload reports for the commit 5a3b4b2 to get more accurate results.

Files Patch % Lines
q2_moshpit/busco/fetch_busco_db.py 87.50% 2 Missing ⚠️
q2_moshpit/busco/busco.py 83.33% 0 Missing and 1 partial ⚠️
q2_moshpit/busco/types/_format.py 95.65% 1 Missing ⚠️
q2_moshpit/busco/utils.py 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #162   +/-   ##
=======================================
  Coverage   96.82%   96.82%           
=======================================
  Files          58       58           
  Lines        3943     3943           
  Branches      364      364           
=======================================
  Hits         3818     3818           
  Misses         80       80           
  Partials       45       45           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Sann5
Copy link
Contributor Author

Sann5 commented May 14, 2024

Ok, @ChristosMatzoros now it's ready. Sorry, this grew so much by adding the rest of the code. It would have been nicer to have the 3 PRs but oh well, it is what it is.

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.

Hey @Sann5,

I had a look in the code and I made some suggestions and comments, please have a look. Everything runs as expected both for downloading the database (fetch-busco-db) and for the visualizer (evaluate-busco). The code is very well structured. Great work!

q2_moshpit/busco/fetch_busco_db.py Outdated Show resolved Hide resolved
q2_moshpit/busco/fetch_busco_db.py Outdated Show resolved Hide resolved
q2_moshpit/busco/fetch_busco_db.py Outdated Show resolved Hide resolved
q2_moshpit/busco/fetch_busco_db.py Outdated Show resolved Hide resolved
q2_moshpit/busco/busco.py Outdated Show resolved Hide resolved
q2_moshpit/plugin_setup.py Outdated Show resolved Hide resolved
q2_moshpit/plugin_setup.py Outdated Show resolved Hide resolved
q2_moshpit/busco/types/_format.py Show resolved Hide resolved
Co-authored-by: Christos Konstantinos Matzoros <[email protected]>
@Sann5
Copy link
Contributor Author

Sann5 commented Jun 6, 2024

@misialq It does not look like the QIIME 2 CI is being triggered.

@misialq
Copy link
Contributor

misialq commented Jun 6, 2024

It doesn't get triggered because I migrated the CI to our new one, which means the Q2 CI only runs when you request it - see my last comment above ;)

@misialq
Copy link
Contributor

misialq commented Jun 6, 2024

/q2ci

Edit: Running QIIME 2 CI...

misialq added 16 commits June 6, 2024 17:49
@misialq misialq merged commit 7ab7588 into bokulich-lab:main Jun 7, 2024
3 checks passed
@lizgehret
Copy link
Contributor

Hey @misialq, just ran a new round of prepares (since we have to patch 2024.5) and ran into this failure in moshpit: https://github.com/qiime2/distributions/actions/runs/9574415150/job/26433191625#step:9:403

I'm not exactly sure what caused these failures (test_evaluate_busco_action for both TestBUSCOFeatureData and TestBUSCOSampleData) but it seems likely to be related to the updates that were made here and here. I can still kick off patch release builds for tiny and amplicon before this needs to be fixed, but wondering if there's anything that stands out to you as being the culprit?

@misialq
Copy link
Contributor

misialq commented Jun 20, 2024

/q2ci

Edit: Running QIIME 2 CI...

@misialq
Copy link
Contributor

misialq commented Jun 20, 2024

Mmmm, good catch @lizgehret, thanks! My guess is that there are some DB files missing among the test files - @Sann5 can you please check?

@Sann5
Copy link
Contributor Author

Sann5 commented Jun 20, 2024

@misialq in deed I think there were some test files missing (e.g. I did not include them in the setup.py). Should I open a PR for this?

@misialq
Copy link
Contributor

misialq commented Jun 20, 2024

Please do, thanks!

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.

4 participants