Suggestions on PR: Calculate per callset stats on gnomAD v4.1 per sample variant counts #614
Conversation
- Make sure to filter to pass and capture variants for most filter options (need to discuss this more with the team)
|
makes sense enough, I think, but let me take another look at it in a bit. and this isn't supposed to add functionality for finding whole callset stats, yeah? just reorganizing and filter changes yeah ? |
Yeah, I haven't gotten to that PR yet, I will add my thoughts for the callset-wide stats to that PR |
Co-authored-by: Qin He <44242118+KoalaQin@users.noreply.github.com> Co-authored-by: Daniel Marten <78616802+matren395@users.noreply.github.com>
Change to use the metadata combinations to determine grouping instead of doing it separately Don't convert the summary stats array to rows, just keep as an array with associated metadata like we do for frequency
…n all possible filter groups was not requested
| - The `filter_group_key_rename` parameter can be used to rename keys in the | ||
| `all_sum_stat_filters`, `common_combo_override`, or `lof_combo_override` | ||
| after creating all combinations. |
There was a problem hiding this comment.
I think this should be:
| - The `filter_group_key_rename` parameter can be used to rename keys in the | |
| `all_sum_stat_filters`, `common_combo_override`, or `lof_combo_override` | |
| after creating all combinations. | |
| - The `filter_group_key_rename` parameter can be used to rename keys in the | |
| generated filter combinations to a different set of keys. |
There was a problem hiding this comment.
no, it renames any key including in the override dictionaries
| common_filter_combos: List[List[str]] = None, | ||
| common_combo_override: Dict[str, List[str]] = None, |
There was a problem hiding this comment.
What do you think of renaming these parameters? I found common_combo cute but it's not very clear.
Maybe like this:
summary_stat_filters: Dict[str, List[str]],
common_filter_combinations: List[List[str]] = None,
common_filter_overrides: Dict[str, List[str]] = None,
lof_filter_combinations: Optional[List[List[str]]] = None,
lof_filter_overrides: Dict[str, List[str]] = None,
filter_key_renames: Dict[str, str] = None,
There was a problem hiding this comment.
I am keeping combinations as combos, it's a common way to shorten combinations similar to how we shorten frequency to freq, and we use "combo" or "combos" in other areas of our code to keep names shorter. I changed to use filter in the override names
| # filter expression from a struct. | ||
| f_expr = filter_exprs.get(f"{k}_{v}") |
There was a problem hiding this comment.
Can you give me an example for f_struct? I get it for loftee_no_flags as f_expr.
Would f_struct for something like maf_af?
There was a problem hiding this comment.
yes, if you look at the gnomad_methods code, it can return a BooleanExpression or a StuctExpression of BooleanExpressions, max_af, csq_set,...
Co-authored-by: Qin He <44242118+KoalaQin@users.noreply.github.com>
…/github.com/broadinstitute/gnomad_qc into jg/move_some_stats_functionality_to_methods
Relies on broadinstitute/gnomad_methods#701
This reorganizes the code to move building most of the filtering expressions in gnomad_methods. There might be more that we can move over to gnomad_methods, but I think we should get these suggestions merged into the main PR first.
I added some more combinations and a general filter for most of the groups to include pass filters and capture filters, but I would like to discuss this more to see what is best for us to do.
I added metadata similar to what we do in frequencies because I think it will be easier to filter to the specific grouping that is wanted.
I also switched to doing the aggregate over an array of the filter groups to prevent a class too large error, and I think also a nice speed-up