-
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: Add --max-length option #1429
Conversation
This is the same as filter-min-length-output-strains.t but using --output-metadata. The contents of --output-metadata are already tested in other tests such as filter-query-example.t.
Repurpose the --min-length test to also include --non-nucleotide.
The purpose of this file is to test --min-length and not other features.
Rename with "min" to pair it with a "max" filter, coming in a following commit.
This complements --min-length.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1429 +/- ##
==========================================
+ Coverage 68.65% 68.66% +0.01%
==========================================
Files 69 69
Lines 7544 7554 +10
Branches 1849 1851 +2
==========================================
+ Hits 5179 5187 +8
- Misses 2088 2089 +1
- Partials 277 278 +1 ☔ View full report in Codecov by Sentry. |
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.
Thank you for adding the new option @victorlin!
I was wondering if filter_by_min_length
and filter_by_max_length
should share some logic, but the separate functions make sense after seeing your previous work to separate the min_date/max_date functions 👍
I assume you mean 8d1d84a - yeah I think it's cleaner to have 1:1 mapping between command-line options and underlying filter functions. The shared logic would only be a few lines anyways. |
Description of proposed changes
Reorganize things related to
--min-length
and add a--max-length
option.Related issue(s)
--max-length
#1421Checklist