-
Notifications
You must be signed in to change notification settings - Fork 2
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 three new parameters to run_amrfinderplus_n #97
ENH: Add three new parameters to run_amrfinderplus_n #97
Conversation
This reverts commit 8378b45.
@misialq could you have a look at the added typemap in this PR.
Also this leads to the parameter gpipe_org not behaving like a bool parameter anymore. So it is not possible to just put --p-gpipe-org but you have to give it a value of eighter True or False. Is this the normal behaviour because I use Bool % Choices(True)? Or is there a better solution? |
Hey @VinzentRisch, I think it's fine for the Int as a placeholder - you are not using it after all. For the boolean, though, I'm not sure if there is a better way - we have it like this somewhere in MOSHPIT and Santiago spent some time exploring this (ending up with a solution like yours) but maybe we could ask tomorrow just to be sure? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @VinzentRisch,
Runs as expected with no errors and code looks good. See a minor change to make and after that we are set to merge.
Thanks for the review, added the full stop :) |
solves #96
Adds three new parameters to run_amrfinderplus_n. Two of those can be used with the action annotate_sample_data_amrfinderplus.
Set up an environment
https://github.com/bokulich-lab/q2-amr?tab=readme-ov-file#installation
Run it locally
Download test files
PR-88.zip
Test it out!