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

Adds --keeplength flag to maff-add action. #84

Merged
merged 2 commits into from
Sep 26, 2024

Conversation

mikerobeson
Copy link
Member

Resolves #83

Allows an option to force the original alignment to keep it's original length while adding new sequences, as outlined here.

Users should keep in mind the following from the mafft docs (which is also in the plugin help text):

If the --keeplength option is given, then the alignment length is unchanged. Insertions at the new sequences are deleted.

@ebolyen
Copy link
Member

ebolyen commented Aug 22, 2024

Hey @mikerobeson! It looks like some of the tests are now failing, should keeplength in the _mafft helper have a default to avoid the issue of the added parameter?

@@ -26,7 +26,8 @@ def run_command(cmd, output_fp, verbose=True):
subprocess.run(cmd, stdout=output_f, check=True)


def _mafft(sequences_fp, alignment_fp, n_threads, parttree, addfragments):
def _mafft(sequences_fp, alignment_fp, n_threads, parttree, addfragments,
keeplength):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
keeplength):
keeplength=False):

Copy link
Member Author

@mikerobeson mikerobeson Aug 22, 2024

Choose a reason for hiding this comment

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

I left this as is for now as it is similar to addfragments. I just I just followed that lead of adding False in the mafft function. I can change that it that is the better way to go. :-)

q2_alignment/_mafft.py Outdated Show resolved Hide resolved
@mikerobeson
Copy link
Member Author

mikerobeson commented Aug 22, 2024

@ebolyen, looks like the check pass now. :-)

Copy link
Member

@gregcaporaso gregcaporaso left a comment

Choose a reason for hiding this comment

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

Looks good @mikerobeson!

@gregcaporaso gregcaporaso merged commit 98741b3 into qiime2:dev Sep 26, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Changelog Needed
Development

Successfully merging this pull request may close these issues.

expose mafft's --keeplength parameter for mafft-add
3 participants