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

Restore withdrawn [VS-581] #8006

Merged
merged 36 commits into from
Sep 7, 2022
Merged

Restore withdrawn [VS-581] #8006

merged 36 commits into from
Sep 7, 2022

Conversation

mcovarr
Copy link
Collaborator

@mcovarr mcovarr commented Aug 29, 2022

Putting this out for early review as I slowly continue to test the changes. 'Hide whitespace' highly recommended when viewing the diffs due to PEP8-fueled horizontal whitespace fixes in a couple of Python scripts.

Screen Shot 2022-08-30 at 8 31 37 AM

@codecov
Copy link

codecov bot commented Aug 29, 2022

Codecov Report

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

Additional details and impacted files
@@               Coverage Diff                @@
##             ah_var_store     #8006   +/-   ##
================================================
  Coverage                ?   86.243%           
  Complexity              ?     35201           
================================================
  Files                   ?      2173           
  Lines                   ?    165004           
  Branches                ?     17791           
================================================
  Hits                    ?    142304           
  Misses                  ?     16373           
  Partials                ?      6327           

@mcovarr mcovarr marked this pull request as ready for review August 30, 2022 12:35
Copy link
Collaborator

@gbggrant gbggrant left a comment

Choose a reason for hiding this comment

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

LGTM


NUMROWS=$(python3 -c "csvObj=open('num_rows.csv','r');csvContents=csvObj.read();print(csvContents.split('\n')[1]);")
SELECT COUNT(*) FROM `~{fq_sample_table}` WHERE
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe the input fq_sample_table_lastmodified_timestamp is no longer needed.

help='File containing list of samples to extract, 1 per line. ' +
'All samples in this file will be included in the cohort regardless of `withdrawn` status in the `sample_info` table.')
sample_args.add_argument('--fq_cohort_sample_names', type=str,
help='FQN of cohort table to extract, contains "sample_name" column. ' +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please spell out FQN (I actually had to look it 😁 )

samples.sample_id NOT IN (SELECT sample_id FROM `~{dataset_name}.sample_load_status` WHERE status="FINISHED") AND
samples.withdrawn is NULL

' > sample_map
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

csv the filename

SELECT IFNULL(MIN(sample_id),0) as min, IFNULL(MAX(sample_id),0) as max FROM `~{dataset_name}.~{table_name}`
AS samples JOIN `~{temp_table}` AS temp ON samples.sample_name = temp.sample_name

' > results
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

csv the filename

@mcovarr mcovarr merged commit 51387f1 into ah_var_store Sep 7, 2022
@mcovarr mcovarr deleted the vs_581_fix_withdrawn branch September 7, 2022 16:02
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.

3 participants