Make several Funcotator methods and fields protected instead of private #8124
Conversation
… to extend the tool
|
@cmnbroad hi chris - sorry to ping you directly here, but does GATK have someone watching PRs? we're hoping to extend funcotator and this PR has some very minor changes from private->protected to enable that. is this something GATK would consider? |
|
@bbimber I'm pinging the funcotator owner who needs to approve this - not sure when he'll respond though. @jonn-smith Thoughts on this ? |
|
@cmnbroad and @jonn-smith Thanks! It now says "First-time contributors need a maintainer to approve running workflows" - does something else need to be approved? I'm not actually a first-time GATK contributor, but I'm not sure how that logic is determined/enforced. I dont have merge privileges' either. @jonn-smith: good to virtually meet you. As I alluded to above, my goal is to extend funcotator to make a new output format that more or less takes the annotations and writes them as discrete INFO fields, rather than the concatenated string. This PR will hopefully largely unblock us. As this develops, do you have any interest in us trying to contribute features back into GATK, or should be just keep separate. There may also be a handful of more minor extensions as we use this more, such as adding a config option to include/exclude specific VCF INFO fields, rather than always transfer 100% of them (which appears to be behavior today, but I just reviewed briefly). Obviously these would be case-by-case, but I'm happy to try to push features back here if you have interest/time in reviewing them. |
There was a problem hiding this comment.
@bbimber This seems harmless to me. It's probably not ideal to subclass funcotator since it's really not designed for it / doesn't have a stable api going forward, but I suspect there's no easy path to do what you want to do to do otherwise.
@jonn-smith Do you have any objections / alternate suggestions?
|
@jonn-smith woops, for some reason I had an ancient tab open here and I didn't notice that you'd already replied. |
|
@lbergelson and @jonn-smith: Thanks and yes, i understand it's not a guaranteed stable tool. If I come up with anything that seems like a candidate change in core funcotator I will reach out here. Tests passed - would someone be able to actually merge the PR? Thanks. |
|
@bbimber Nice to virtually meet you too. I think we've probably interacted once or twice in the past as well. I'm glad to hear you're planning on using and extending Funcotator! It would be great if you could push back the useful changes you make. FYI - come the new year, I'm going to be refactoring several things in the Funcotator core that will make it much more extensible (mostly around GTF parsing). I'm also going to fix a few bugs involving variants that span transcript and contig boundaries. It's been a long time coming - I've just been working on a set of high-priority tasks that needed my attention. Also - Funcotator is production-ready for human data, so it should basically be "guaranteed stable" (modulo certain known issues like the ones I referenced above). |
|
@jonn-smith Thanks. My lab actually works with macaque data and one driver of this project is building a custom data source. The tweaks to formatting and discrete INFO fields is primarily about making the results easier to view in standard genome browsers. I'll be working on this in Jan and will try to push changes back here if they seem to make sense. |
|
@bbimber Ah, very nice. Yeah - readability of the annotations has always been an issue. I usually convert the output to TSV/MAF to visually inspect fields after running it (in the code there are other options). FYI, The parser updates I need to make are so that Funcotator better supports other species. It's currently too restrictive on the GTF files used as the primary transcriptome references. |
|
@jonn-smith, yeah, I noticed the restrictive GTF parsing. I "solved" this by using a dummy GTF with one fake line. For our purposes we care about transferring annotations, not really about categorizing their coding potential (which already happened upstream). |
|
@jonn-smith do you know if there is a nightly build of GATK pushed to a maven repo anymore? I dont see it here: https://broadinstitute.jfrog.io/ui/native/libs-snapshot/org/broadinstitute/gatk/, nor in libs-snapshot-local. |
Hello - we're interested in creating a custom extension of Funcotator with different output formats. This PR should be quite low risk - it just converts a handful of privates fields/methods to protected to make it easier to extend this tool.