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

Add more gVCF support #991

Merged
merged 16 commits into from Mar 27, 2018

Conversation

Projects
None yet
2 participants
@cmdcolin
Contributor

cmdcolin commented Feb 15, 2018

This PR attempts to add more support for GVCF, breaking from #847 to allow that PR to be more single topic

Most gVCF files consists of blocks of "<NON_REF>" calls that span a range (e.g. it uses the END= info field).

Some special cases

  1. escapeHTML is done to make the <NON_REF> type things printable in popup
  2. reference_allele is ignored when there is END field because it appears this just takes on a junk value of a single dna letter
  3. alternative_allele added even for these symbolic alleles, note that they can take the form of a list e.g. G,<NON_REF>. For this, note that the check for if(alt) that was added in #847 now goes away

Several sample data test files are added, including gatk haplotype caller run in gvcf mode run over the volvox-sorted bam file :)

@wafflebot wafflebot bot added the in progress label Feb 15, 2018

@rbuels rbuels added this to the 1.13.0 milestone Feb 27, 2018

@rbuels rbuels changed the base branch from parse_cnv_end_vcf to dev Feb 27, 2018

@rbuels rbuels modified the milestones: 1.13.0, 1.13.1 Mar 14, 2018

@rbuels

This looks good, except for the HTML-entitizing of all detail fields. I think that needs to be somehow limited to VCF details

@@ -187,8 +189,7 @@ return declare( null, {
return keys.length;
}
}
domConstruct.create('div', { className: 'value '+class_, innerHTML: val }, parent );
domConstruct.create('div', { className: 'value '+class_, innerHTML: dojoxHtmlEntities.encode(''+val) }, parent );

This comment has been minimized.

@rbuels

rbuels Mar 26, 2018

Collaborator

I don't think this is a good idea, entity-izing ALL feature details. People out there are probably relying on being able to put HTML in their feature attributes and have it render.

Perhaps something could be added to VariantDetailMixin to accomplish this?

This comment has been minimized.

@cmdcolin

cmdcolin Mar 26, 2018

Contributor

I guess the solution could just be to remove the <> in the alt field in the first place, but it sort of helps to signal to the end user what it came from

it('parses END field', function() {
var store = new VCFStore({

This comment has been minimized.

@rbuels

rbuels Mar 26, 2018

Collaborator

oh god I love these so much. thank you thank you thank you for adding some

@rbuels

This comment has been minimized.

Collaborator

rbuels commented Mar 27, 2018

I reverted the html-entitizing and the display didn't look any different to me, but maybe I'm looking at the wrong thing.

anyway, a little-known feature of the detailmixin is: if a thing (any data type) has a toHTML method on it, the detail page will call it and display that HTML. So, you could add toHTML methods to some things in VariantDetailMixin, maybe?

@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented Mar 27, 2018

The <*> actually doesn't cause problems but other symbolic alleles do including DEL:ME in the volvox test data and <NON_REF> from gvcf and other things. Note that on the current live jbrowse.org instance, it just displays 1|1 in the genotype field on the DEL:ME in the genotype table, but with this PR it tries to pull the actual alt allele text (as it should IMO) into the genotype table

Here's a screenshot displaying the issue

The encoding entities nicely fixed both the the alternative allele field and the genotype table but possibly could look for another workaround

screenshot-localhost-2018 03 27-11-42-01

@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented Mar 27, 2018

Looks like the reason that the dgrid was fixed also was that renderDetailValue got called on the data columns of the dgrid as well as those regular detail value fields

@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented Mar 27, 2018

I fixed the rendering on the genotype table but didn't get the alternative_alleles field to render (couldn't get the toHTML function working)

@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented Mar 27, 2018

Code should now escape only the alternative_alleles field in popup as requested :)

@rbuels

This comment has been minimized.

Collaborator

rbuels commented Mar 27, 2018

awesome, nice job!

@rbuels rbuels merged commit 0dad87a into dev Mar 27, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@cmdcolin cmdcolin deleted the gvcf_support branch Apr 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment