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 fetch-eggnog-hmmer-db action #173

Merged
merged 38 commits into from
Jul 3, 2024

Conversation

Sann5
Copy link
Contributor

@Sann5 Sann5 commented May 23, 2024

What's new

  • An action that fetches a HMMER database. This will be used downstream to find seed orthologues (to be implemented).
  • Closes ENH: add fetch-hmmer-db action #167
  • ❌ CI tests should fail until a version of q2-types with the PR mentioned below is up in conda

Blocked by...

Preview Give feedback

Run it locally

  1. Checkout the PR branch.

Assuming 1) you already have a local copy of q2-moshpit that is installed in editable mode in your activated virtual environment and 2) that "origin" is the name of the remote of this GitHub repo; run the following.

cd my_q2-moshpit
PR=173
git fetch origin pull/${PR}/head:pr-${PR}
git checkout pr-${PR}

If the PR in q2-types mentioned above is not yet merged you will need to do the same for q2-types. E.g:

cd my_q2-types
PR=328
git fetch origin pull/${PR}/head:pr-${PR}
git checkout pr-${PR}
  1. Test it out!
qiime moshpit fetch-eggnog-hmmer-db --p-taxon-id 1100069 --output-dir 1100069_db --verbose

@Sann5 Sann5 requested a review from a team May 23, 2024 14:32
@Sann5 Sann5 self-assigned this May 23, 2024
@Sann5 Sann5 requested review from VinzentRisch and removed request for a team May 23, 2024 14:32
@VinzentRisch
Copy link
Contributor

Hi @Sann5
I don't know if i have time to look at it tomorrow. Maybe I'll only look at it next week. Hope that is not a problem :)

Copy link
Contributor

@VinzentRisch VinzentRisch left a comment

Choose a reason for hiding this comment

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

Looks very good!
I ran it and it works with no problems.
See some minor changes and typos below. :)

q2_moshpit/eggnog/_dbs.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_dbs.py Outdated Show resolved Hide resolved
q2_moshpit/plugin_setup.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/tests/test_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/tests/test_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/tests/test_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
@Sann5 Sann5 requested a review from VinzentRisch May 27, 2024 14:59
Copy link
Contributor

@VinzentRisch VinzentRisch left a comment

Choose a reason for hiding this comment

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

Everything looks good! :)

@Sann5 Sann5 requested a review from VinzentRisch June 11, 2024 14:05
@Sann5
Copy link
Contributor Author

Sann5 commented Jun 11, 2024

I made quite a bit of changes @VinzentRisch since there were changes required in the dependent PR. could you give it a another look before i merge?

Copy link
Contributor

@misialq misialq 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 think this still needs a bit of work - see below.

q2_moshpit/eggnog/_format.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.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
@Sann5 Sann5 requested a review from misialq June 21, 2024 13:57
Copy link
Contributor

@misialq misialq 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, thanks for those changes! The error handling is much better now 🥇 See some more comments below.

q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/_utils.py Show resolved Hide resolved
q2_moshpit/plugin_setup.py Outdated Show resolved Hide resolved
q2_moshpit/plugin_setup.py Outdated Show resolved Hide resolved
@Sann5 Sann5 requested a review from misialq June 25, 2024 12:00
q2_moshpit/eggnog/tests/test_dbs.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/tests/test_utils.py Outdated Show resolved Hide resolved
q2_moshpit/eggnog/tests/test_utils.py Show resolved Hide resolved
Copy link

This PR/issue depends on:

@Sann5 Sann5 requested a review from misialq June 26, 2024 13:19
@misialq misialq merged commit ab391cd into bokulich-lab:main Jul 3, 2024
2 of 3 checks passed
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.

ENH: add fetch-hmmer-db action
3 participants