Adding metrics for estimating strand specificity. #733

nh13 · 2017-01-14T21:04:24Z

Added two metrics that measure the fraction of observed reads that
support R1 being on the strand of transcription versus the opposite
strand of transcription. For paired end reads, R2 is assumed to be on
the opposite strand of that of R1.

coveralls · 2017-01-14T21:12:38Z

Coverage increased (+0.02%) to 71.149% when pulling 78bfc7f on nh_report_strand_specificity into 1007816 on master.

tfenne

@nh13 I think this looks much better. I do have some minor, and probably easily addressed concerns about the actual algorithm, though in fairness some of those pre-date your changes!!

I think it would be better to use the strand of the read you are observing in your loop - which just makes the check marginally more complicated - rather than the strand reported for R1 from the record you have.

If you were to want to tighten things up in general, another way to allay that concern would be to filter for all the strand-specificity checks, to only inserts that are fully contained within the span of a gene (and with the existing check for the read overlapping an exon). That way you are guaranteed when you look at the mate pair's strand, that the mate pair is also within the gene's locus, and not you're not looking at a pair that happens to be overlap the start/end of the gene by chance. Gene even has a start and end field populated correctly that would make that check trivial. FWIW though this will probably have minimal effect on any reasonable library.

tfenne · 2017-01-15T11:50:00Z

src/main/java/picard/analysis/RnaSeqMetrics.java

@@ -66,6 +66,19 @@
    /** Number of aligned reads that are mapped to the incorrect strand.  0 if library is not strand-specific. */
    public long INCORRECT_STRAND_READS;

+    /** The fraction of reads that support the model where R1 is on the strand of transcription and R2 is on the
+     * opposite strand of transcription.  For unpaired reads, this is the fraction of reads that are on the same strand


I would just stop the sentence at "opposite strand."

tfenne · 2017-01-15T11:59:46Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java


+                if (!rec.getReadPairedFlag() || SamPairUtil.getPairOrientation(rec) == SamPairUtil.PairOrientation.FR) {


I believe this will fail with reads that have unmapped mates, which I can't find being filtered out above. SamPairUtil.getPairOrientation will throw an IllegalArgumentException for mate-paired reads with an unmapped mate.

tfenne · 2017-01-15T12:01:51Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java

+                if (!rec.getReadPairedFlag() || SamPairUtil.getPairOrientation(rec) == SamPairUtil.PairOrientation.FR) {
+                    final boolean firstReadNegativeStrand;
+                    // Get the strand of the first read.
+                    if (!rec.getReadPairedFlag() || rec.getFirstOfPairFlag()) firstReadNegativeStrand = rec.getReadNegativeStrandFlag();


There are a couple of things here that seem a little off to me:

You're looking at first of pair strand for your test, but you only really know whether the read you have in hand (maybe R1 or R2) overlaps an exon.

You're only looking at R1 strand, but you're single-counting it for fragments and double-counting it for pairs.

nh13 · 2017-01-15T22:05:15Z

@tfenne care to take one more look?

coveralls · 2017-01-15T22:13:38Z

Coverage increased (+0.03%) to 71.158% when pulling 01dd00b on nh_report_strand_specificity into 1007816 on master.

tfenne

Looks good to me. One minor suggestion. 👍

tfenne · 2017-01-16T12:55:20Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java

+                        else {
+                            properOrientation    = SamPairUtil.getPairOrientation(rec) == SamPairUtil.PairOrientation.FR;
+                            leftMostAlignedBase  = Math.min(rec.getAlignmentStart(), rec.getMateAlignmentStart());
+                            rightMostAlignedBase = Math.max(rec.getAlignmentEnd(), SAMUtils.getMateAlignmentEnd(rec)); // requires the mate cigar


I think you could just use the inferred insert size here to avoid requiring the mate cigar.

I think this is true for FR reads, but not FF reads. Unfortunately, Picard/htsjdk computes insert size as 5' to 5' rather than left-most to right-most bases.

"Picard/htsjdk computes insert size"? I was under the impression that it leaves it alone as it was placed by the mapper. Can you point to the code that does this?

An example is SamPairUtil.computeInsertSize within SamPairUtil.setMateInfo which is used in AbstractAlignmentMerger.transferAlignmentInfoToPairedRead.

nh13 · 2017-01-16T20:31:01Z

@yfarjoun it would be great to have a look by someone else, either yourself, or someone you suggest?

yfarjoun

Thanks @nh13 This looks very nice.

I have some comments about the implementation and tests. I realize that some of the tests I requested were missing but given that you changed some of the filters (getNotPrimaryAlignmentFlag) I'd like to be sure that we're not missing something...

yfarjoun · 2017-01-17T01:08:44Z

src/main/java/picard/analysis/RnaSeqMetrics.java

@@ -66,6 +66,19 @@
    /** Number of aligned reads that are mapped to the incorrect strand.  0 if library is not strand-specific. */
    public long INCORRECT_STRAND_READS;

+    /** The fraction of reads that support the model where R1 is on the strand of transcription and R2 is on the
+     * opposite strand.  For unpaired reads, this is the fraction of reads that are on the same strand than that of the


"For unpaired reads, it is the fraction of read that are on the transcription strand (out of all the reads)."

yfarjoun · 2017-01-17T01:09:02Z

src/main/java/picard/analysis/RnaSeqMetrics.java

+
+    /**
+     * The fraction of reads that support the model where R2 is on the strand of transcription and R1 is on the opposite
+     * strand.  For unpaired reads, this is the fraction of reads that are on the opposite strand than that of the


yfarjoun · 2017-01-17T01:19:52Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java

@@ -232,25 +228,60 @@ public void acceptRecord(SAMRecord rec) {

            // Strand-specificity is tallied on read basis rather than base at a time.  A read that aligns to more than one
            // gene is not counted.
-            if (!rec.getNotPrimaryAlignmentFlag() && overlapsExon && strandSpecificity != StrandSpecificity.NONE && overlappingGenes.size() == 1) {


will this now start counting secondary and supplementary alignments as full reads?

See line 140 (https://github.com/broadinstitute/picard/pull/733/files#diff-5c77d4872231d96dbaeb66da41c02f84R140) where there is a return statement if rec.getNotPrimaryAlignmentFlag().

yfarjoun · 2017-01-17T01:22:16Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java

+                        if (rec.getMateUnmappedFlag()) {
+                            properOrientation   = false; // mate is unmapped!
+                            leftMostAlignedBase = rightMostAlignedBase = 0;
+                        }


yfarjoun · 2017-01-17T01:32:58Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java

+                        else {
+                            properOrientation    = SamPairUtil.getPairOrientation(rec) == SamPairUtil.PairOrientation.FR;
+                            leftMostAlignedBase  = Math.min(rec.getAlignmentStart(), rec.getMateAlignmentStart());
+                            rightMostAlignedBase = Math.max(rec.getAlignmentEnd(), SAMUtils.getMateAlignmentEnd(rec)); // requires the mate cigar


This will blow-up without MC tag won't it? If so that's a change in spec, since I think this collector didn't need to have MC in the past...so I'd prefer if you could use a poorer solution with a warning in case you find a record with no MC tag.

for example use the inferred read length if the mate cigar is missing...

Done, I'll fight the battle to change your mind at a latter date. I am going to omit a warning, since then I'd have to find a way to pass a log in, and I'm not up for that.

yfarjoun · 2017-01-17T02:15:10Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

+                "REF_FLAT=" +            getRefFlatFile(sequence).getAbsolutePath(),
+                "RIBOSOMAL_INTERVALS=" + rRnaIntervalsFile.getAbsolutePath(),
+                "STRAND_SPECIFICITY=SECOND_READ_TRANSCRIPTION_STRAND",
+                "IGNORE_SEQUENCE=" +ignoredSequence


space after +

Fixed here and elsewhere (copy paste)

yfarjoun · 2017-01-17T02:19:45Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

+        builder.addPair("read_one_end_unmapped", sequenceIndex, 50, 51, false, true, "50M", null, false, false, -1);
+
+        // Reads that are enclosed in the gene, paired and frag: one count per template
+        builder.addFrag("frag_enclosed_forward", sequenceIndex, 150, false);


These have null cigars...so how many bases get counted towards coverage?

answered my own question. 36.

yfarjoun · 2017-01-17T02:20:29Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

+        final int sequenceIndex = builder.getHeader().getSequenceIndex(sequence);
+
+        // Reads that start *before* the gene: not counted
+        builder.addPair("pair_prior_1", sequenceIndex, 45, 50, false, false, "50M", "50M", true, false, -1);


could you please add:

secondary alignments

supplementary alignments

records that do not have MC tag

duplicate reads.

ignored sequence reads

non-PF reads

I did not add 5 since that is tested in the other tests.

FYI, duplicate reads are OK.

NB: previously were were including supplementary reads when calculating CORRECT_STRAND_READS and INCORRECT_STRAND_READS. Now we are not.

yfarjoun · 2017-01-17T02:27:40Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

+
+        final RnaSeqMetrics metrics = output.getMetrics().get(0);
+        Assert.assertEquals(metrics.PF_ALIGNED_BASES, 722);
+        Assert.assertEquals(metrics.PF_BASES, 576);


is it just me or is it strange that PF_BASES < PF_ALIGNED_BASES?

I had to set the read length on the builder, the call to add* didn't check the cigar versus the bases length (yikes).

yfarjoun · 2017-01-17T02:31:27Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

+        samFile.deleteOnExit();
+
+        final SAMFileWriter samWriter = new SAMFileWriterFactory().makeSAMWriter(builder.getHeader(), false, samFile);
+        for (final SAMRecord rec: builder.getRecords()) samWriter.addAlignment(rec);


Streams for the win?!?

Same as the rest of the code. Keeping it for now. I am not in love with Java streams yet 👍

nh13

@yfarjoun addressed your concerns.

nh13 · 2017-01-17T02:49:03Z

src/main/java/picard/analysis/RnaSeqMetrics.java

@@ -66,6 +66,19 @@
    /** Number of aligned reads that are mapped to the incorrect strand.  0 if library is not strand-specific. */
    public long INCORRECT_STRAND_READS;

+    /** The fraction of reads that support the model where R1 is on the strand of transcription and R2 is on the
+     * opposite strand.  For unpaired reads, this is the fraction of reads that are on the same strand than that of the


nh13 · 2017-01-17T02:49:52Z

src/main/java/picard/analysis/RnaSeqMetrics.java

+
+    /**
+     * The fraction of reads that support the model where R2 is on the strand of transcription and R1 is on the opposite
+     * strand.  For unpaired reads, this is the fraction of reads that are on the opposite strand than that of the


nh13 · 2017-01-17T02:51:00Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java

@@ -232,25 +228,60 @@ public void acceptRecord(SAMRecord rec) {

            // Strand-specificity is tallied on read basis rather than base at a time.  A read that aligns to more than one
            // gene is not counted.
-            if (!rec.getNotPrimaryAlignmentFlag() && overlapsExon && strandSpecificity != StrandSpecificity.NONE && overlappingGenes.size() == 1) {


See line 140 (https://github.com/broadinstitute/picard/pull/733/files#diff-5c77d4872231d96dbaeb66da41c02f84R140) where there is a return statement if rec.getNotPrimaryAlignmentFlag().

nh13 · 2017-01-17T02:52:15Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java

+                        if (rec.getMateUnmappedFlag()) {
+                            properOrientation   = false; // mate is unmapped!
+                            leftMostAlignedBase = rightMostAlignedBase = 0;
+                        }


nh13 · 2017-01-17T02:54:42Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java

+                        else {
+                            properOrientation    = SamPairUtil.getPairOrientation(rec) == SamPairUtil.PairOrientation.FR;
+                            leftMostAlignedBase  = Math.min(rec.getAlignmentStart(), rec.getMateAlignmentStart());
+                            rightMostAlignedBase = Math.max(rec.getAlignmentEnd(), SAMUtils.getMateAlignmentEnd(rec)); // requires the mate cigar


An example is SamPairUtil.computeInsertSize within SamPairUtil.setMateInfo which is used in AbstractAlignmentMerger.transferAlignmentInfoToPairedRead.

nh13 · 2017-01-17T03:14:02Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

+                "REF_FLAT=" +            getRefFlatFile(sequence).getAbsolutePath(),
+                "RIBOSOMAL_INTERVALS=" + rRnaIntervalsFile.getAbsolutePath(),
+                "STRAND_SPECIFICITY=SECOND_READ_TRANSCRIPTION_STRAND",
+                "IGNORE_SEQUENCE=" +ignoredSequence


Fixed here and elsewhere (copy paste)

nh13 · 2017-01-17T03:19:41Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

+        final int sequenceIndex = builder.getHeader().getSequenceIndex(sequence);
+
+        // Reads that start *before* the gene: not counted
+        builder.addPair("pair_prior_1", sequenceIndex, 45, 50, false, false, "50M", "50M", true, false, -1);


I did not add 5 since that is tested in the other tests.

nh13 · 2017-01-17T03:37:10Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

+        final int sequenceIndex = builder.getHeader().getSequenceIndex(sequence);
+
+        // Reads that start *before* the gene: not counted
+        builder.addPair("pair_prior_1", sequenceIndex, 45, 50, false, false, "50M", "50M", true, false, -1);


FYI, duplicate reads are OK.

nh13 · 2017-01-17T03:39:09Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

+        final int sequenceIndex = builder.getHeader().getSequenceIndex(sequence);
+
+        // Reads that start *before* the gene: not counted
+        builder.addPair("pair_prior_1", sequenceIndex, 45, 50, false, false, "50M", "50M", true, false, -1);


NB: previously were were including supplementary reads when calculating CORRECT_STRAND_READS and INCORRECT_STRAND_READS. Now we are not.

nh13 · 2017-01-17T03:45:45Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

+
+        final RnaSeqMetrics metrics = output.getMetrics().get(0);
+        Assert.assertEquals(metrics.PF_ALIGNED_BASES, 722);
+        Assert.assertEquals(metrics.PF_BASES, 576);


I had to set the read length on the builder, the call to add* didn't check the cigar versus the bases length (yikes).

nh13 · 2017-01-17T03:46:23Z

@yfarjoun thanks for the helpful comments, take another look and let me know what you think.

coveralls · 2017-01-17T03:53:20Z

Coverage increased (+0.04%) to 71.175% when pulling 9428cef on nh_report_strand_specificity into 1007816 on master.

yfarjoun

minor comments. thanks!

yfarjoun · 2017-01-17T04:19:08Z

src/main/java/picard/analysis/RnaSeqMetrics.java

+    /**
+     * The fraction of reads for which the transcription strand model could not be inferred.
+     */
+    public long NUM_UNEXPLAINED_TRANSCRIPTION_STRAND_READS;


inconclusive rather than unexplained?

yfarjoun · 2017-01-17T04:19:26Z

src/main/java/picard/analysis/RnaSeqMetrics.java

     */
    public double PCT_FIRST_READ_TRANSCRIPTION_STRAND_READS;

    /**
     * The fraction of reads that support the model where R2 is on the strand of transcription and R1 is on the opposite
-     * strand.  For unpaired reads, this is the fraction of reads that are on the opposite strand than that of the
-     * transcription strand.
+     * strand.  For unpaired reads, it is the fraction of read that are on opposite strand than that of the the


~~the~~ the

yfarjoun · 2017-01-17T04:19:40Z

src/main/java/picard/analysis/RnaSeqMetrics.java

+
+    /**
+     * The fraction of reads that support the model where R2 is on the strand of transcription and R1 is on the opposite
+     * strand.  For unpaired reads, it is the fraction of read that are on opposite strand than that of the the


~~the~~ the

fraction->number (x2)

This is still worded in a confusing manner (for unpaired reads)

Removing unpaired description as it is just plain confusing.

yfarjoun · 2017-01-17T04:20:04Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java

@@ -226,9 +226,13 @@ public void acceptRecord(SAMRecord rec) {
                }
            }

+            if (metrics.PF_ALIGNED_BASES > metrics.PF_BASES) {
+                throw new IllegalStateException("HUH?");


yfarjoun · 2017-01-17T04:21:19Z

src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java

+                            // Get the alignment length of the mate.  Approximate it with the current read length if no
+                            // mate cigar is found.
+                            final Cigar mateCigar         = SAMUtils.getMateCigar(rec);
+                            final int mateReferenceLength = (mateCigar == null) ? rec.getReadLength() : mateCigar.getReferenceLength();


thanks for humoring me.

yfarjoun · 2017-01-17T04:25:52Z

src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java

        builder.addPair("pair_prior_1", sequenceIndex, 45, 50, false, false, "50M", "50M", true, false, -1);
        builder.addPair("pair_prior_2", sequenceIndex, 49, 50, false, false, "50M", "50M", true, false, -1);

-        // Read that is enclosed in the gene, but one end does not map: not counted
+        // Read that is enclosed in the gene, but one end does not map: not counted: count as unexamined


~~not counted:~~

yfarjoun · 2017-01-17T04:45:58Z

src/main/java/picard/analysis/RnaSeqMetrics.java

+    public long NUM_SECOND_READ_TRANSCRIPTION_STRAND_READS;
+
+    /**
+     * The fraction of reads for which the transcription strand model could not be inferred.


fraction->number

yfarjoun · 2017-01-17T04:50:22Z

src/main/java/picard/analysis/RnaSeqMetrics.java

@@ -66,19 +66,43 @@
    /** Number of aligned reads that are mapped to the incorrect strand.  0 if library is not strand-specific. */
    public long INCORRECT_STRAND_READS;

+    /** The number of reads that support the model where R1 is on the strand of transcription and R2 is on the
+     * opposite strand.  For unpaired reads, it is the fraction of read that are on the transcription strand (out of all


tfenne · 2017-01-19T12:50:50Z

@nh13 I just realized that somewhere between my review and @yfarjoun's review something changed that I disagree with. Previously the two percentages were calculated using the sum of the NUM_FIRST_READ_TRANSCRIPTION_STRAND_READS and NUM_SECOND_READ_TRANSCRIPTION_STRAND_READS alone. Now the denominator also rolls in NUM_UNEXPLAINED_TRANSCRIPTION_STRAND_READS.

I think this is wrong because what that represents is reads that aren't properly paired or in the expected FR orientation, so we can't really assay them for strandedness. I think it should go back to just using the two values for the denominator.

Also, while I'm here, could we change FIRST_READ to R1 and SECOND_READ to R2 for brevity please?

nh13 · 2017-01-19T16:33:21Z

@tfenne thanks for catching this, I'll update.

nh13 · 2017-01-19T18:06:04Z

@yfarjoun and @tfenne, thanks for the comments. I feel I have all of the resolved, so one final 👍 and I can merge after tests pass.

coveralls · 2017-01-19T18:14:51Z

Coverage increased (+0.05%) to 71.179% when pulling 82f83b4 on nh_report_strand_specificity into 1007816 on master.

tfenne · 2017-01-19T21:49:51Z

👍 from me. @yfarjoun ?

yfarjoun · 2017-01-20T01:58:53Z

actually, given that oracle DB has problems with long names...could you shorten the length of the metrics names to 30 or less?

yfarjoun · 2017-01-20T01:59:43Z

sorry for the late comment. (I was starting to think about the needed DB changes..)

nh13 · 2017-01-24T18:06:14Z

@yfarjoun I shortened them to 30 characters or less. 👍 , then I can rebase and once tests pass merge?

coveralls · 2017-01-24T18:14:50Z

Coverage increased (+0.09%) to 71.223% when pulling 37f496d on nh_report_strand_specificity into 1007816 on master.

Added two metrics that measure the fraction of observed reads that support R1 being on the strand of transcription versus the opposite strand of transcription. For paired end reads, R2 is assumed to be on the opposite strand of that of R1.

coveralls · 2017-01-24T19:38:51Z

Coverage increased (+0.04%) to 71.223% when pulling 86682fb on nh_report_strand_specificity into 3800c82 on master.

nh13 assigned tfenne Jan 14, 2017

nh13 requested a review from tfenne January 14, 2017 21:04

tfenne requested changes Jan 15, 2017

View reviewed changes

tfenne assigned nh13 and unassigned tfenne Jan 15, 2017

nh13 assigned tfenne and unassigned nh13 Jan 15, 2017

tfenne approved these changes Jan 16, 2017

View reviewed changes

tfenne assigned nh13 and unassigned tfenne Jan 16, 2017

nh13 requested a review from yfarjoun January 16, 2017 20:30

nh13 assigned yfarjoun and unassigned nh13 Jan 16, 2017

yfarjoun suggested changes Jan 17, 2017

View reviewed changes

nh13 commented Jan 17, 2017

View reviewed changes

yfarjoun approved these changes Jan 18, 2017

View reviewed changes

nh13 unassigned yfarjoun Jan 23, 2017

nh13 self-assigned this Jan 23, 2017

nh13 assigned yfarjoun and unassigned nh13 Jan 24, 2017

nh13 force-pushed the nh_report_strand_specificity branch from 37f496d to 86682fb Compare January 24, 2017 19:30

nh13 merged commit 1c6cd8a into master Jan 24, 2017

nh13 deleted the nh_report_strand_specificity branch January 24, 2017 19:43


		if (!rec.getReadPairedFlag() \|\| SamPairUtil.getPairOrientation(rec) == SamPairUtil.PairOrientation.FR) {

Adding metrics for estimating strand specificity. #733

Adding metrics for estimating strand specificity. #733

Conversation

nh13 commented Jan 14, 2017

coveralls commented Jan 14, 2017

tfenne left a 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

nh13 commented Jan 15, 2017

coveralls commented Jan 15, 2017

tfenne left a 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

nh13 commented Jan 16, 2017

yfarjoun left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nh13 left a 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

Choose a reason for hiding this comment

nh13 commented Jan 17, 2017

coveralls commented Jan 17, 2017

yfarjoun left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tfenne commented Jan 19, 2017

nh13 commented Jan 19, 2017

nh13 commented Jan 19, 2017

coveralls commented Jan 19, 2017

tfenne commented Jan 19, 2017

yfarjoun commented Jan 20, 2017

yfarjoun commented Jan 20, 2017

nh13 commented Jan 24, 2017