-
Notifications
You must be signed in to change notification settings - Fork 129
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
filter: Update --min-length
help message
#1422
Conversation
Should I also update the Line 431 in 14f5ce4
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1422 +/- ##
=======================================
Coverage 68.50% 68.50%
=======================================
Files 69 69
Lines 7537 7537
Branches 1846 1846
=======================================
Hits 5163 5163
Misses 2091 2091
Partials 283 283 ☔ View full report in Codecov by Sentry. |
That is unintuitive! I think good to clarify in both the help and report messages. |
Maybe also worth noting that it's case-insensitive. |
Explicitly state that the minimum length only counts the standard nucleotide characters A, C, G, or T (case-insensitive). This has been the behavior since version 3.0.3.dev1, but has never been explicitly documented outside of the `filter_by_sequence_length` docstring.
623ecd4
to
b86e997
Compare
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.
lgtm, thanks!
Description of proposed changes
Explicitly state that the minimum length only counts the standard nucleotide characters (A, C, G, or T).
This has been the behavior since version 3.0.3.dev1, but has never been explicitly documented outside of the
filter_by_sequence_length
docstring.Related issue(s)
Prompted by @j23414's conversation in nextstrain/dengue#28 (comment)
Checklist