-
Notifications
You must be signed in to change notification settings - Fork 46
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
Fix show matching causatives #4412
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4412 +/- ##
==========================================
- Coverage 84.70% 84.69% -0.01%
==========================================
Files 310 310
Lines 18413 18412 -1
==========================================
- Hits 15596 15594 -2
- Misses 2817 2818 +1 ☔ View full report in Codecov by Sentry. |
…-Genomics/scout into fix_matching_causatives
…-Genomics/scout into fix_matching_causatives
Sorry. x_x |
I also feel immediately guilty when it's code I've touched.. And given that I've touched most of the code in scout I feel always guilty (and in general I am guilty) 🤣 |
@@ -1397,13 +1397,7 @@ def _matching_causatives( | |||
The subset of all secondary findings found in default gene panels | |||
) | |||
""" | |||
matching_causatives_filter = list( | |||
set(other_causatives_filter + other_causatives_in_default_panels_filter) |
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.
In the specific case of the bug other_causatives_filter = [] because there are no panels in the safe search for the institute:
other_causatives_in_default_panels_filter comes from this function:
https://github.com/Clinical-Genomics/scout/blob/413d8301c84b4d070578ab53b7aed6a3ccb3f093/scout/server/blueprints/cases/controllers.py#L389C34-L389C64
I believe that that function is returning the list of genes in the case default panels, which is too limiting for searching for matching causatives in all cases of the institute
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.
🥇
matching_causatives = store.case_matching_causatives( | ||
case_obj=case_obj, limit_genes=matching_causatives_filter | ||
) | ||
matching_causatives = store.case_matching_causatives(case_obj=case_obj) |
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.
🥇 In the end it is safer and probably faster really with current indexes.
We conclude it was deviously hard to see, and a bit of the classic Python “[] is False” issue.
In this case a non-transitive version of it were give_me_them([]) + give_me_them([1, 2]) != give_me_them([] +[1, 2])
.
In summary, we were likely without matching causatives outside the default panel from Oct. 😞 At any rate, don't blame yourself @alkc - we looked at the PR as well without realising, and it did not easily reproduce on local when I tested.
@@ -570,7 +570,7 @@ def case_matching_causatives(self, case_obj, limit_genes=None): | |||
0:4 | |||
] # example: [ "17", "7577559", "G" "A"] | |||
|
|||
for variant_type in ["clinical", "reseach"]: | |||
for variant_type in ["clinical", "research"]: |
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.
🐛 🐛 🐛
CHANGELOG.md
Outdated
@@ -27,6 +27,7 @@ About changelog [here](https://keepachangelog.com/en/1.0.0/) | |||
- Rename `Clinical significanc` to `Germline classification` in ClinVar submissions exported files | |||
- Rename `Comment on clinical significance` to `Comment on classification` in ClinVar submissions exported files | |||
- Show matching partial causatives on variant page | |||
- Matching causatives query |
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.
Probably add a slightly meatier explanation, including more exactly what was wrong and perhaps a date-range as well. The functionality is mostly for extra safety and speed, there is a world where we kind of rely on it to pick up less obvious phenotypes.
Quality Gate passedIssues Measures |
This PR adds a functionality or fixes a bug.
Test with these variants:
Solved case --> https://scout-stage.scilifelab.se/cust002/F0015136
Variant not shown in:
Prepare for testing
scout-stage
and the server iscg-vm1
.ssh <USER.NAME>@cg-vm1.scilifelab.se
sudo -iu hiseq.clinical
ssh localhost
podman ps
systemctl --user stop scout.target
systemctl --user start scout@<this_branch>
systemctl --user status scout.target
scout-stage
) to be used for testing by other users.Testing on hasta server (Clinical Genomics Stockholm)
Prepare for testing
ssh <USER.NAME>@hasta.scilifelab.se
us; paxa -u <user> -s hasta -r scout-stage
. You can also use the WSGI Pax app available at https://pax.scilifelab.se/.conda activate S_scout; pip freeze | grep scout-browser
bash /home/proj/production/servers/resources/hasta.scilifelab.se/update-tool-stage.sh -e S_scout -t scout -b <this_branch>
us; scout --version
paxa
procedure, which will release the allocated resource (scout-stage
) to be used for testing by other users.How to test:
Expected outcome:
The functionality should be working
Take a screenshot and attach or copy/paste the output.
Review: