-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix partner variant bug #486
base: main
Are you sure you want to change the base?
Conversation
self.pedigree.by_id[sample_id].affected != '2' | ||
or not principal.sample_category_check(sample_id, allow_support=False) |
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.
none of this ended up being relevant, but it makes the logic easier to read
# thin out the possible partners by alt depth | ||
return [ | ||
var | ||
for var in comp_hets[sample].get(first_variant, []) | ||
if not var.check_minimum_alt_depth(sample, min_alt_depth) | ||
] |
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 think this was the issue - I was getting a Bool value for if the current variant was support-only, then using that to decide if a partner could also be support-only. It was a mess of booleans, so I've dropped it from here completely. Once we find all the partners, there's a simpler test on each pair that at least one must be non-support.
return [ | ||
var | ||
for var in comp_hets[sample].get(first_variant, []) | ||
if not var.check_minimum_alt_depth(sample, min_alt_depth) |
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 know it is not part of this PR, but the check_minimum_alt_depth
method name is a bit curly (ie the not
in front of it confused me)
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.
Yup, I think I should revisit all the method names to optimise the logic they're involved in...
Fixes
Proposed Changes
Checklist