remove withdrawn req #7844
remove withdrawn req #7844
Conversation
Codecov Report
|
|
As part of this, do we want to also remove this restriction in the other places we put it in? A quick grep turns up GvsExtractCallset.wdl |
|
Bec and I were just having this discussion---do we want to just unblock CH asap or do we want to remove this everywhere (esp since we might just be swapping the word "withdrawn" with "eradicated"---but given that we haven't unblocked him yet and seem to be blocked on the date / timestamp---yes, let's remove them all! |
| meta { | ||
| volatile: true | ||
| } | ||
|
|
There was a problem hiding this comment.
do we want to retain this?
There was a problem hiding this comment.
I think you should. It's entirely dependent on the database state.
There was a problem hiding this comment.
ok---no longer removed as part of the pr!
| meta { | ||
| volatile: true | ||
| } | ||
|
|
There was a problem hiding this comment.
I think you should. It's entirely dependent on the database state.
| meta { | ||
| volatile: true | ||
| } | ||
|
|
| @@ -128,7 +128,7 @@ task CheckForDuplicateData { | |||
| echo "WITH items as (SELECT s.sample_id, s.sample_name, s.is_loaded, s.withdrawn FROM \`${TEMP_TABLE}\` t left outer join \`${SAMPLE_INFO_TABLE}\` s on (s.sample_name = t.sample_name)) " >> query.sql | |||
| echo "SELECT i.sample_name FROM \`${INFO_SCHEMA_TABLE}\` p JOIN items i ON (p.partition_id = CAST(i.sample_id AS STRING)) WHERE p.total_logical_bytes > 0 AND (table_name like 'ref_ranges_%' OR table_name like 'vet_%')" >> query.sql | |||
| echo "UNION DISTINCT " >> query.sql | |||
| echo "SELECT i.sample_name FROM items i WHERE i.is_loaded = True AND i.withdrawn IS NULL " >> query.sql | |||
| echo "SELECT i.sample_name FROM items i WHERE i.is_loaded = True " >> query.sql | |||
There was a problem hiding this comment.
Oh this is a good fix -- previously we would have allowed loading in a duplicate sample IF the one in the db was withdrawn
| @@ -89,7 +89,7 @@ def get_all_sample_ids(fq_destination_table_samples): | |||
| def create_extract_samples_table(control_samples, fq_destination_table_samples, fq_sample_name_table, fq_sample_mapping_table): | |||
| sql = f"CREATE OR REPLACE TABLE `{fq_destination_table_samples}` AS (" \ | |||
| f"SELECT m.sample_id, m.sample_name, m.is_loaded, m.withdrawn, m.is_control FROM `{fq_sample_name_table}` s JOIN `{fq_sample_mapping_table}` m ON (s.sample_name = m.sample_name) " \ | |||
| f"WHERE m.is_loaded is TRUE AND m.withdrawn IS NULL AND m.is_control = {control_samples})" | |||
| f"WHERE m.is_loaded is TRUE AND m.is_control = {control_samples})" | |||
There was a problem hiding this comment.
separate discussion, but I wonder if this encoding of "control" samples is another case where we put biz logic into GVS?
There was a problem hiding this comment.
maybe---but the biz logic is OURS for sensitivity and precision
RoriCremer
force-pushed
the
rc-vs-418-allow-withdrawn
branch
from
cbda9de
to
af32fe8
Compare
|
other than manually testing this---is there a test I can write to include this? Maybe add it to the QS integration test? |
Allow CH to create custom cohorts with a subset of samples
Tested with sample_id=1 (sample name = ERS4367795)
Imported:
In the alt-allele
In the filter:
In the prepare:
In the extract:
