-
Notifications
You must be signed in to change notification settings - Fork 590
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
Treat withdrawn samples in sub-cohort prepare correctly [VS-772] #8156
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## ah_var_store #8156 +/- ##
================================================
Coverage ? 86.177%
Complexity ? 35138
================================================
Files ? 2173
Lines ? 165045
Branches ? 17794
================================================
Hits ? 142231
Misses ? 16470
Partials ? 6344 |
@@ -89,12 +89,12 @@ def get_all_sample_ids(fq_destination_table_samples, only_output_vet_tables, fq_ | |||
|
|||
|
|||
def create_extract_samples_table(control_samples, fq_destination_table_samples, fq_sample_name_table, | |||
fq_sample_mapping_table, honor_withdrawn: bool): | |||
fq_sample_mapping_table, honor_withdrawn): |
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.
was there a reason to remove the type ascription?
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.
I didn't understand why it was there and nowhere else, and it worked without it.
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.
Yeah IMHO we probably should use these type hints liberally; I'm also unsure why this was the exception rather than the rule. Perhaps something to discuss going forward for coding standards.
These type hints are not enforced by the Python runtime so code will work the same with or without them. They're intended mostly for the benefit of developers so a Python-aware IDE can warn if an actual parameter of an incompatible type is being passed.
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.
LGTM
No description provided.