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

NEW: Exposes strand parameter for cluster_features_de_novo (#90) #90

Merged
merged 5 commits into from
Oct 13, 2023
Merged

NEW: Exposes strand parameter for cluster_features_de_novo (#90) #90

merged 5 commits into from
Oct 13, 2023

Conversation

vaamb
Copy link
Contributor

@vaamb vaamb commented May 9, 2023

Can be usefull when working with mixed-orientation sequences in order to merge together sequences with their reverse complement prior to other analyses

Used the same default parameters and description than for the 2 other cluster_features_*

@lizgehret
Copy link
Member

Apologies for the delayed review on this @vaamb - implementation looks good, but it would be nice to add some associated unit tests to confirm that this added parameter is functioning as expected!

@lizgehret
Copy link
Member

Hey @vaamb we'll have to bump this to the next release if you aren't able to get unit tests added by EOD tomorrow. Thanks!

@vaamb vaamb marked this pull request as draft October 13, 2023 14:55
- Used the same default parameters and description than the 2 other `cluster_features_*`
- It was a copy and paste of the comment under `test_skip_denovo`
…and_no_clustering` and `test_99_percent_clustering_strand`
@vaamb vaamb marked this pull request as ready for review October 13, 2023 16:26
@vaamb
Copy link
Contributor Author

vaamb commented Oct 13, 2023

@lizgehret I finally found the time to add the tests

@lizgehret lizgehret self-requested a review October 13, 2023 19:24
Copy link
Member

@lizgehret lizgehret left a comment

Choose a reason for hiding this comment

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

These tests look good, thanks @vaamb! Will go ahead and get this merged!

@lizgehret lizgehret merged commit 693d1c2 into qiime2:dev Oct 13, 2023
4 checks passed
@lizgehret lizgehret changed the title Expose strand parameter for cluster_features_de_novo NEW: Exposes strand parameter for cluster_features_de_novo (#90) Oct 13, 2023
@vaamb vaamb deleted the expose_strand_parameter branch October 13, 2023 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

2 participants