-
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
Better ClinVar tooltips and added exclude clinsig status for SNVs filter #4993
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4993 +/- ##
==========================================
- Coverage 84.78% 84.78% -0.01%
==========================================
Files 323 323
Lines 19432 19435 +3
==========================================
+ Hits 16475 16477 +2
- Misses 2957 2958 +1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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.
Super with the tooltip!
Looks nice, but I have a few issues with the functionality?
First, the exclude now seems exclusive, e.g. removing variants without clinvar hits. That is not intended. One would like to whittle down the list of variants, say avoiding seeing the ones that are known Benign or Likely benign, but seeing the rest.
Exclusion filtering on my local still seems bugged?
I'll give it the benefit of the doubt if it works on stage (I have been messing with setup quite a bit) but I think we are not quite there yet?
I also still haven't quite wrapped my head around what would be supposed to happen if you check exclude and high significance, but giving the high significance priority is needed for clincal filter, so all good.
@@ -113,6 +113,7 @@ class VariantFiltersForm(FlaskForm): | |||
compound_follow_filter = BooleanField("Compounds follow filter") | |||
cadd_inclusive = BooleanField("CADD inclusive") | |||
clinsig = NonValidatingSelectMultipleField("ClinVar CLINSIG", choices=CLINSIG_OPTIONS) | |||
clinsig_exclude = BooleanField("Exclude significance") |
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.
Optionally drop "significance"; should be as clear from context and tooltip.
scout/adapter/mongo/query.py
Outdated
{"revstat": re.compile("|".join(trusted_revision_level))}, | ||
] | ||
} | ||
} | ||
} | ||
else: | ||
LOG.debug("add CLINSIG filter for rank: %s" % ", ".join(str(query["clinsig"]))) | ||
clnsig_query = {"clnsig": {"$elemMatch": {"$or": elem_match_value}}} |
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.
Keep missing value!
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.
Good point, I'll fix, thanks!
Co-authored-by: Daniel Nilsson <daniel.k.nilsson@gmail.com>
|
||
Returns: | ||
clinsig_query(dict): a dictionary with clinsig key-values | ||
def clinsig_query(self, query: dict) -> dict: |
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.
This function is only refactored, but it returns the same dict as before
I'll wait for this one until we have #5006 in place, because they are touching the same code |
Quality Gate passedIssues Measures |
This PR adds a functionality or fixes a bug.
New checkbox
Better tooltip
New tooltip
Note that this PR is only covering SNVs filters, I plan to add the exclude filter to the other categories in another PR (I think it's going to be easier to review this way)
Testing on cg-vm1 server (Clinical Genomics Stockholm)
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: