-
Notifications
You must be signed in to change notification settings - Fork 596
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
clearer error when values are missing #7939
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## ah_var_store #7939 +/- ##
================================================
Coverage ? 86.241%
Complexity ? 35201
================================================
Files ? 2173
Lines ? 165016
Branches ? 17792
================================================
Hits ? 142311
Misses ? 16378
Partials ? 6327 |
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.
Nits picked, looks good but some comments for posterity would be great 👍
@@ -990,29 +990,38 @@ task ClinvarSignificance { | |||
# "other", | |||
# "not provided"] | |||
|
|||
bq query --nouse_legacy_sql --project_id=~{query_project_id} --format=csv 'SELECT | |||
bq query --nouse_legacy_sql --project_id=~{query_project_id} --format=csv 'SELECT |
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.
🔙
|
||
echo "false" > ~{pf_file} | ||
# if the result of the query has any rows, that means gvs_all_an has not been calculated correctly | ||
if [[ $NUMRESULTS -ge 13 && $INCLUVALUES = "0" ]]; then | ||
# if the result of the query less than 13 rows, that means clinvar_classification must not have all the expected values |
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.
# if the result of the query less than 13 rows, that means clinvar_classification must not have all the expected values | |
# If the result of the query has fewer than 13 rows, that means clinvar_classification must not have all the expected values. |
# if the result of the query has any rows, that means gvs_all_an has not been calculated correctly | ||
if [[ $NUMRESULTS -ge 13 && $INCLUVALUES = "0" ]]; then | ||
# if the result of the query less than 13 rows, that means clinvar_classification must not have all the expected values | ||
if [[ $NUMRESULTS -ge 13 && $NUMMISS = "0" ]]; then |
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.
if [[ $NUMRESULTS -ge 13 && $NUMMISS = "0" ]]; then | |
if [[ $NUMRESULTS -ge 13 && $NUMMISS -eq 0 ]]; then |
index($0,"not provided"))}' bq_clinvar_classes.csv) | ||
~{fq_vat_table}, UNNEST(clinvar_classification) AS unnested_clinvar_classification'| sed "2 d" > bq_clinvar_classes.csv | ||
|
||
echo "affects" >> expected_clinvar_classes.csv |
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.
TOL this could be DRYed
echo 'affects
association
benign
.
.
.
uncertain significance' > expected_clinvar_classes.csv
f12853c
to
7238988
Compare
In the VAT validation, give clearer error msg about which clinvar classification values are missing