Added Variants and Features GA4GH, with GA4GH example file #464

akmorrow13 · 2017-07-04T23:20:40Z

This PR contains the following:

GA4GH Variants
GA4GV Features
Features Viz Track
GA4GH Example File for Features and Variants

What's left for this PR:

Add a GA4GH Reads example on the GA4GH example page.
There is an issue with using sinon server and dataCanvas at the same time. Therefore, the tests for FeatureTrack will fail. We can either add in the Feature JSON and rewrite the tests on the JSON datasource, or add these tests in a separate PR once the JSON formats are done.
Test in python to verify GA4GH schemas

This change is

armish

Thanks a lot for all these changes, @akmorrow13! I have a few minor comments here and there but this looks great overall!

re: failing tests: I would rather not have any of them failing if you can think of a quick workaround for the JSON issue. I also don't want this to be blocked by the JSON features, so you can decide on the way to work around the issue in a way that won't take too much of your time and won't compromise the tests for now.

Again - thanks a lot for the amazing work! Can't wait to make a new release featuring all of these new additions.

armish · 2017-07-06T01:52:55Z

README.md

@@ -98,6 +98,8 @@ To play with the demo, start an [http-server][hs]:

 Then open [http://localhost:8080/examples/index.html](http://localhost:8080/examples/index.html) in your browser of choice.

+To view integration with GA4GH schemas, view [http://localhost:8080/examples/ga4gh-example.html](http://localhost:8080/examples/ga4gh-example.html).


Kudos for isolating all these GA4GH integration parts into a separate example 🙇

armish · 2017-07-06T01:56:26Z

examples/data-ga4gh.js

+  {
+    viz: pileup.viz.variants(),
+    data: pileup.formats.GAVariant({
+        endpoint: 'http://1kgenomes.ga4gh.org',


would you mind extracting this and the following endpoint as a variable that is defined at the top of the file with some pointers to the 1kgenomes server and a simple explanation to why this server is special? That would make it easier for a random person to compare this with the standard data example.

armish · 2017-07-06T02:05:33Z

src/main/Interval.js

+  // Expand range to begin and end on multiples of size
+  expand(size: number, zeroBased: boolean): Interval {
+    var minimum = zeroBased ? 0 : 1;
+    var roundDown = x => x - x % size;


can you add a simple example to what this line would do as a comment? And maybe encapsulate the modulo operation in parentheses just for clarity?

This is just me thinking loud: the expand name seems a bit misleading here: I first thought that this was just expanding the region by the given size, but this might actually not expand one of the ends if either the start or stop are already multiples of a size - e.g.: (10, 20) -> expand(5) -> (10, 20). Maybe using round in the name would be better but I leave it up to you since this was what it was named originally.

Ok sounds good! I will rename it. Also, I am still not sure on the zeroBased parameter. Another option is changing zeroBased to an integer, and passing that in as the minimum. So instead you would have:

round(size: number, minimum: number): Interval { var roundDown = x => x - x % size; var newStart = Math.max(minimum, roundDown(this.start)), newStop = roundDown(this.stop + size - 1); return new Interval(newStart, newStop); }

This is pretty minor, but it could be cleaner.

armish · 2017-07-06T02:09:03Z

src/main/sources/BamDataSource.js

@@ -77,7 +70,7 @@ function createFromBamFile(remoteSource: BamFile): AlignmentDataSource {
        return Q.when();
      }

-      interval = expandRange(interval);
+      interval = interval.expand(BASE_PAIRS_PER_FETCH, ZERO_BASED);


armish · 2017-07-06T02:11:01Z

src/main/sources/BigBedDataSource.js

 // Flow type for export.
 export type BigBedSource = {
  rangeChanged: (newRange: GenomeRange) => void;
-  getGenesInRange: (range: ContigInterval<string>) => Gene[];
+  getFeaturesInRange: (range: ContigInterval<string>) => Gene[];


thanks for renaming this — feature would better speak to the bioinformaticians in this context.

armish · 2017-07-06T02:12:43Z

src/main/sources/GA4GHAlignmentSource.js

@@ -52,6 +41,9 @@ function create(spec: GA4GHSpec): AlignmentDataSource {
  var coveredRanges: ContigInterval<string>[] = [];

  function addReadsFromResponse(response: Object) {
+    if (response.alignments === undefined) {
+      return;


curious why/when this happens. I would have expected the alignments to be an empty array for the semantics of it, but is this how GA4GH server responds when no reads match the criteria?

This occurs when you query a region without data. For example, for variants, if you query the GA4GH server:
curl --data '{"variantSetId": "WyIxa2dlbm9tZXMiLCJ2cyIsInBoYXNlMy1yZWxlYXNlIl0", "end": "10200", "pageSize": "0", "pageToken": "", "start": "10000", "callSetIds": ["WyIxa2dlbm9tZXMiLCJ2cyIsInBoYXNlMy1yZWxlYXNlIiwiSEcwMDA5NiJd"], "referenceName": "10"}' --header 'Content-Type: application/json' http://1kgenomes.ga4gh.org/variants/search

It will return {}

Therefore, if trying to call .variants (or .alignments in the alignment case), on the empty JSON object, the code will error.

armish · 2017-07-06T02:17:13Z

src/main/viz/VariantTrack.js

@@ -126,7 +126,7 @@ class VariantTrack extends React.Component {
    if (variants && variants.length>0) {
      var data = [];
      for (var i=0;i<variants.length;i++) {
-        data.push({id: variants[i].id,vcfLine:variants[i].vcfLine});
+        data.push({id:variants[i].id,vcfLine:variants[i].vcfLine,ref:variants[i].ref,alt:variants[i].alt});


sorry for the styling inconsistency here; if you had a chance, would you mind fixing the style here? OK if you don't get to it:

// ... for (var i=0; i < variants.length; i++) { data.push({ id: variants[i].id, vcfLine: variants[i].vcfLine, ref: variants[i].ref, alt: variants[i].alt }); } // ...

Styling nits are appreciated! Will fix!

coveralls · 2017-07-11T06:12:05Z

Coverage decreased (-0.3%) to 90.663% when pulling a0c0a25 on akmorrow13:otherGA4GH into 3859912 on hammerlab:master.

akmorrow13 · 2017-07-12T05:38:42Z

@armish Thanks for all the comments! I decided that it would be easiest to just add the JSON stuff required to pass the FeatureTrack tests, so those should all be in! Let me know if there are any other comments we should address.

One thing to note is the Flow issues: In VcfDataSource.js and BigBedDataSource.js I had to add an export flag to make Flow happy with some imports. Although this is not the cleanest solution, it seems to be a bug with jsxhint that can only be fixed this way.

coveralls · 2017-07-12T05:41:29Z

Coverage decreased (-0.3%) to 90.663% when pulling 0bff6d2 on akmorrow13:otherGA4GH into 3859912 on hammerlab:master.

armish · 2017-07-23T19:51:26Z

thank you, @akmorrow13! LGTM.

armish reviewed Jul 6, 2017

View reviewed changes

akmorrow13 force-pushed the otherGA4GH branch from ef97cfb to e18039a Compare July 6, 2017 06:57

akmorrow13 added 3 commits July 11, 2017 22:34

Added variants and features GA4GH API

0a8a47e

Added required JSON for passing FeatureTrack tests

ec64209

Finished GA4GH example page

0bff6d2

akmorrow13 force-pushed the otherGA4GH branch from a0c0a25 to 0bff6d2 Compare July 12, 2017 05:35

armish merged commit 5c78282 into hammerlab:master Jul 23, 2017

This was referenced Jul 27, 2017

Next npm publish date #466

Closed

add JSON format for GA4GH variants and features #423

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Variants and Features GA4GH, with GA4GH example file #464

Added Variants and Features GA4GH, with GA4GH example file #464

akmorrow13 commented Jul 4, 2017 •

edited

Loading

armish left a comment •

edited

Loading

armish Jul 6, 2017

armish Jul 6, 2017

armish Jul 6, 2017

akmorrow13 Jul 6, 2017

armish Jul 6, 2017

armish Jul 6, 2017

armish Jul 6, 2017

akmorrow13 Jul 6, 2017

armish Jul 6, 2017

akmorrow13 Jul 6, 2017

coveralls commented Jul 11, 2017

akmorrow13 commented Jul 12, 2017

coveralls commented Jul 12, 2017

armish commented Jul 23, 2017

		@@ -98,6 +98,8 @@ To play with the demo, start an [http-server][hs]:

		Then open [http://localhost:8080/examples/index.html](http://localhost:8080/examples/index.html) in your browser of choice.

		To view integration with GA4GH schemas, view [http://localhost:8080/examples/ga4gh-example.html](http://localhost:8080/examples/ga4gh-example.html).

Added Variants and Features GA4GH, with GA4GH example file #464

Added Variants and Features GA4GH, with GA4GH example file #464

Conversation

akmorrow13 commented Jul 4, 2017 • edited Loading

armish left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jul 11, 2017

akmorrow13 commented Jul 12, 2017

coveralls commented Jul 12, 2017

armish commented Jul 23, 2017

akmorrow13 commented Jul 4, 2017 •

edited

Loading

armish left a comment •

edited

Loading