-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: add option to disable normal hardfilter #1509
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1509 +/- ##
===========================================
+ Coverage 99.48% 99.50% +0.02%
===========================================
Files 40 40
Lines 1932 2036 +104
===========================================
+ Hits 1922 2026 +104
Misses 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
This PR isn't finished yet in terms of all the testing and documentation, but I can take some reviews on the code! |
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.
First round of reviews. I think the way how filters and configurations are set is quite messy. It would be nice to try to clean up as much as possible to make it more readable. In my opinion it would be nicer to use classes instead, but the classes should be properly defined and with a clear scope so there is no confusion between them. It is already a bit confusing the VariantCallerFilters and VarCallerFilters. It would be good to either merge, rename or refactor to know what is going on
|
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.
Nice improvements! 🌟 Just some questions below
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.
Thanks for addressing it. I'm still unsure if there would be any unexpected behaviour from the code. But it seems that you have tested this and it's good to go. So 🚀
Thanks a lot for the review and the great suggestions @fevac ! ❤️ |
Description
Some customers have requested a feature to get better control over the normal filtration in Scout. We can achieve this by transforming the matched normal filter to as soft-filter that can be optionally applied in Scout.
Linked MTP-BALSAMIC story: https://github.com/Clinical-Genomics/MTP-BALSAMIC/issues/1
To enable this feature we need to do two more things:
After applying this soft filter argument there will be 2 new filters appearing in the final VCF:
Added
--soft-filter-normal
to disable hard filtration of matched normal filtersChanged
Documentation
Tests
Feature Tests
Ensure that all workflows are still filtering properly, with or without the new soft-filter normal option. See google sheet: https://docs.google.com/spreadsheets/d/15oU6JoLUybFwY76iRDOnvnEDuybjDElhu07oB3V6vR0/edit?gid=0#gid=0
With soft-filter
TN TGA
TO TGA
TN WES
TO WES
TN UMI
TO UMI
TN WGS
TO WGS
Without soft-filter
TN TGA
TO TGA
TN WES
TO WES
TN UMI
TO UMI
TN WGS
TO WGS
Pipeline still works after changing filter name to "in_normal", sanity check
TN TGA completes successfully
Pipeline Integrity Tests
.hk
file)Clinical Genomics Stockholm
Documentation
Panel of Normal specific criteria
User Changes
Infrastructure Changes
Checklist
Important
Ensure that all checkboxes below are ticked before merging.
For Developers
For Reviewers
conditions where applicable, with satisfactory results.