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

fixed top_priority_effect for ExonicSpliceSite #235

Merged

Conversation

iskandr
Copy link
Contributor

@iskandr iskandr commented Oct 9, 2018

Currently, if an ExonicSpliceSite effect has an alternate_effect which is more interesting (e.g. StopGain), it doesn't come up as the top priority. This changes the top_priority_effect logic to return the alternate_effect if it's higher priority than a potential exonic splice site change.

Other changes:

  • moved splicing mutations to be right after Silent in the effect ordering.
  • fixed bug I noticed in VCF writing where a random hg19 FASTA path was always used as the reference

Fixes: #233

@coveralls
Copy link

coveralls commented Oct 9, 2018

Coverage Status

Coverage decreased (-0.07%) to 87.393% when pulling d1e7fd3 on fix-effect-ordering-and-top-priority-for-exonic-splice-site into 408b1cf on master.

IntronicSpliceSite,
# exonic variants near a splice boundary
ExonicSpliceSite,
# modification or deletion of stop codon
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

@@ -125,13 +124,31 @@ def effect_sort_key(effect):
effect_class_priority = effect_priority(effect)
return (effect_class_priority, cds_length, transcript_length)


def select_between_exonic_splice_site_and_alternate_effect(effect):
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add some comments here? this functionality is kinda non-obvious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a docstring

@@ -248,9 +249,11 @@ def get_sample_names():
headers += ['FORMAT'] + sample_names

unique_variants_list = merge_duplicate_variants()
# can't have more than one reference in VCF so pick the first one and pray
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of praying, throw an error? when would we ever want to support combining VCFs created against different references?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm imagining hg19 vs. GRCh37 type differences but you're probably right: better to raise an exception

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to check for a unique reference

@iskandr iskandr merged commit 61cb6b7 into master Oct 12, 2018
@iskandr iskandr deleted the fix-effect-ordering-and-top-priority-for-exonic-splice-site branch October 12, 2018 20:01
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.

Consider alternative effect of ExonicSpliceSite for "top priority"
3 participants