Skip to content
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

Fail Avro extract and callset stats on bad filter name [VS-655] #8046

Merged
merged 9 commits into from
Oct 7, 2022

Conversation

mcovarr
Copy link
Collaborator

@mcovarr mcovarr commented Oct 5, 2022

FAILED runs with bad filter names:

Successful runs with good filter names:

Copy link

@rsasch rsasch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's already a task ValidateFilterSetName in GvsExtractCallset (see https://github.com/broadinstitute/gatk/blob/ah_var_store/scripts/variantstore/wdl/GvsExtractCallset.wdl#L117); perhaps these two tasks could be combined?

@codecov
Copy link

codecov bot commented Oct 5, 2022

Codecov Report

❗ No coverage uploaded for pull request base (ah_var_store@b088a5c). Click here to learn what that means.
The diff coverage is n/a.

Additional details and impacted files
@@               Coverage Diff                @@
##             ah_var_store     #8046   +/-   ##
================================================
  Coverage                ?   86.249%           
  Complexity              ?     35205           
================================================
  Files                   ?      2173           
  Lines                   ?    165004           
  Branches                ?     17791           
================================================
  Hits                    ?    142314           
  Misses                  ?     16362           
  Partials                ?      6328           

@mcovarr mcovarr requested a review from rsasch October 5, 2022 20:16
Copy link

@rsasch rsasch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one small nit and, assuming you add links to test runs of GvsExtractCallset 👍🏻

@mcovarr
Copy link
Collaborator Author

mcovarr commented Oct 5, 2022

I've got a GvsQuickstartIntegration running on this now... 🤞

https://app.terra.bio/#workspaces/gvs-dev/mlc%20GVS%20Quickstart%20v3/job_history/06c71819-8aa7-48ec-aa1d-12326a0efe6b

@mcovarr mcovarr force-pushed the vs_655_avro_extract_warn_on_bad_filter_name branch from 9e961b4 to 76da384 Compare October 6, 2022 15:36
@@ -11,7 +11,16 @@ workflow GvsExtractAvroFilesForHail {
Int scatter_width = 10
}

call OutputPath { input: go = true }
call Utils.ValidateFilterSetName {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smart to reuse this!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rsasch gets the credit for suggesting this! 😄

@@ -207,52 +207,11 @@ workflow GvsExtractCallset {
}
}

task ValidateFilterSetName {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇


echo "project_id = ~{query_project}" > ~/.bigqueryrc

OUTPUT=$(bq --location=US --project_id=~{query_project} --format=csv query --use_legacy_sql=false ~{bq_labels} "SELECT filter_set_name as available_filter_set_names FROM \`~{data_project}.~{data_dataset}.filter_set_info\` GROUP BY filter_set_name")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind seeing if this works without the "--location=US" part and, if so, removing it? As is, it'll fail if we specify a dataset anywhere else. I'm 99.99% sure this call will function perfectly without that parameter.

@mcovarr mcovarr merged commit 01b2880 into ah_var_store Oct 7, 2022
@mcovarr mcovarr deleted the vs_655_avro_extract_warn_on_bad_filter_name branch October 7, 2022 14:23
This was referenced Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants