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: Added ability to specify segment coverage value for extract-seq-segments. #199

Merged
merged 4 commits into from
Aug 14, 2024

Conversation

mikerobeson
Copy link
Collaborator

Adresses #195

This PR adds the option --p-target-coverage to extract-seq-segments. This denotes the amount of coverage the sequence segments must have in order to extract the corresponding match from a longer sequence.

Copy link
Collaborator

@nbokulich nbokulich left a comment

Choose a reason for hiding this comment

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

Hey @mikerobeson looks good to me, I just have a minor comment inline. Thanks!

@@ -1141,6 +1144,8 @@
parameter_descriptions={
'threads': VSEARCH_PARAM_DESCRIPTIONS['threads'],
'perc_identity': VSEARCH_PARAM_DESCRIPTIONS['perc_identity'],
'target_coverage': 'Minimum coverage of reference segment '
Copy link
Collaborator

Choose a reason for hiding this comment

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

This param description is a little confusing. Could you please make it clearer?

Also since this is target_coverage, shouldn't this be the minimum coverage of the target, which is the reference sequence segments in this case? As written now it sounds like this is coverage of the input sequences.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've updated the help text to be more clear.

Copy link
Collaborator

@nbokulich nbokulich left a comment

Choose a reason for hiding this comment

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

thanks @mikerobeson ! Looks great

@nbokulich nbokulich merged commit 4f7ab4b into bokulich-lab:master Aug 14, 2024
4 checks passed
@mikerobeson mikerobeson deleted the min-coverage branch August 14, 2024 11:57
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.

2 participants