brentp / geneimpacts

prioritize effects of variant annotations from VEP, SnpEff, et al.
MIT License
32 stars 15 forks source link

Add prioritizing effects by canonical status with VEP. #13

Closed roryk closed 6 years ago

roryk commented 6 years ago

This adds a flag which alters the sort order of VEP-annotated variants to prioritize the effect with the canonical flag set above all other considerations.

I added some tests for this and it seems to work correctly and not alter the default behavior. Let me know what you think.

brentp commented 6 years ago

on quick look:

what do you think about naming the kwarg canonical_first or prioritize_canonical? instead of report_canonical.

Also, I think adding an argument that's specific to VEP will complicate things downstream. I wonder if adding this to the Effect base-class and making it a no-op would be better. Then, in gemini, we can just send that argument (if passed) to any Effect class. This would also be useful if, e.g. bcftools BCSQ reports the canonical transcript. not sure if it does.

roryk commented 6 years ago

Great suggestions, I'll swap over the naming and move things so it will work more transparently downstream.

roryk commented 6 years ago

I couldn't think of a great way to add it to the base class, so I just stuck it as an argument for all of the derived classes, where it does nothing. Better suggestions would be great if you have them.

brentp commented 6 years ago

thanks!

roryk commented 6 years ago

Thanks Brent!

brentp commented 6 years ago

thank you! I also tagged a new version and sent it to pypi.