Merge freq array function and new frequency dictionary builder #551
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mike-w-wilson
commented
Jun 14, 2023
Fix incorrect formatting
mike-w-wilson
commented
Jun 14, 2023
mike-w-wilson
commented
Jun 14, 2023
mike-w-wilson
commented
Jun 14, 2023
Correcting incorrect formatting again...
mike-w-wilson
commented
Jun 14, 2023
mike-w-wilson
commented
Jun 14, 2023
jkgoodrich
requested changes
Jun 20, 2023
gnomad/utils/annotations.py
Outdated
Comment on lines
1309
to
1318
| AC=hl.case() | ||
| .when(set_negatives_to_zero, hl.max(x.AC, 0)) | ||
| .or_error(negative_value_error_msg), | ||
| AN=hl.case() | ||
| .when(set_negatives_to_zero, hl.max(x.AN, 0)) | ||
| .or_error(negative_value_error_msg), | ||
| homozygote_count=hl.case() | ||
| .when(set_negatives_to_zero, hl.max(x.homozygote_count, 0)) | ||
| .or_error(negative_value_error_msg), | ||
| ) |
There was a problem hiding this comment.
**{
ann: hl.case()
.when(set_negatives_to_zero, hl.max(x[ann], 0))
.or_error(negative_value_error_msg)
for ann in callstat_ann
}
Co-authored-by: jkgoodrich <33063077+jkgoodrich@users.noreply.github.com>
mike-w-wilson
commented
Jun 21, 2023
gnomad/utils/annotations.py
Outdated
| @@ -1106,7 +1108,7 @@ def region_flag_expr( | |||
| :return: `region_flag` struct row annotation | |||
| """ | |||
| prob_flags_expr = ( | |||
| {"non_par": (t.locus.in_x_nonpar() | t.locus.in_y_nonpar())} if non_par else {} | |||
| {"non_par": t.locus.in_x_nonpar() | t.locus.in_y_nonpar()} if non_par else {} | |||
There was a problem hiding this comment.
I'm not sure how to stop black from doing this...this format "correction" will not work with hail
There was a problem hiding this comment.
oh really? I didn't realize hail doesn't like that. Maybe add # fmt: skip to the end of it?
mike-w-wilson
commented
Jun 21, 2023
|
Back to you @jkgoodrich ! |
jkgoodrich
requested changes
Jun 23, 2023
gnomad/utils/annotations.py
Outdated
| @@ -1106,7 +1108,7 @@ def region_flag_expr( | |||
| :return: `region_flag` struct row annotation | |||
| """ | |||
| prob_flags_expr = ( | |||
| {"non_par": (t.locus.in_x_nonpar() | t.locus.in_y_nonpar())} if non_par else {} | |||
| {"non_par": t.locus.in_x_nonpar() | t.locus.in_y_nonpar()} if non_par else {} | |||
There was a problem hiding this comment.
oh really? I didn't realize hail doesn't like that. Maybe add # fmt: skip to the end of it?
gnomad/utils/annotations.py
Outdated
| :param fmeta: List of frequency metadata for arrays being merged. | ||
| :param operation: Merge operation to perform. Options are "sum" and "diff". If "diff" is passed, the first freq array in the list will have the other arrays subtracted from it. | ||
| :param set_negatives_to_zero: If True, set negative array values to 0 for AC, AN, AF, and homozygote_count. If False, raise a ValueError. Default is True. | ||
| :return: Tuple of merged frequency array and its freq index dictionary. |
There was a problem hiding this comment.
It's the freq meta list no instead of freq index dictionary right?
Co-authored-by: jkgoodrich <33063077+jkgoodrich@users.noreply.github.com>
mike-w-wilson
commented
Jun 23, 2023
| @@ -1106,7 +1108,8 @@ def region_flag_expr( | |||
| :return: `region_flag` struct row annotation | |||
| """ | |||
| prob_flags_expr = ( | |||
| {"non_par": (t.locus.in_x_nonpar() | t.locus.in_y_nonpar())} if non_par else {} | |||
| {"non_par": (t.locus.in_x_nonpar() | t.locus.in_y_nonpar()) | |||
| } if non_par else {} # fmt: skip | |||
There was a problem hiding this comment.
Ha the extra note made the line long enough that black wants parentheses there now...
There was a problem hiding this comment.
oh boy. I'm surprised it was even removing the parentheses, mine doesn't do that
jkgoodrich
approved these changes
Jun 23, 2023
| @@ -1106,7 +1108,8 @@ def region_flag_expr( | |||
| :return: `region_flag` struct row annotation | |||
| """ | |||
| prob_flags_expr = ( | |||
| {"non_par": (t.locus.in_x_nonpar() | t.locus.in_y_nonpar())} if non_par else {} | |||
| {"non_par": (t.locus.in_x_nonpar() | t.locus.in_y_nonpar()) | |||
| } if non_par else {} # fmt: skip | |||
There was a problem hiding this comment.
oh boy. I'm surprised it was even removing the parentheses, mine doesn't do that
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds the ability to merge multiple frequency arrays that are on the same table. The merge supports addition or subtraction of arrays. For subtraction, all freq arrays are subtracted from the first freq array in the list. This also adds a new function to build frequency array dictionaries from the frequency metadata.