[DISQ-10] Initial Disq code contribution. #14

tomwhite · 2018-07-19T16:14:19Z

Corresponds to tomwhite@d3d76ec.

heuermh · 2018-07-19T16:20:35Z

Fixes #10.

magicDGS

Review classes in org.disq_bio.disq package. I did pass through htsjdk.samtools too, but without getting into details because that will be ported to htsjdk.

magicDGS · 2018-07-19T17:12:04Z

src/main/java/htsjdk/samtools/BAMFileReader2.java

+import java.util.NoSuchElementException;
+
+/** Class for reading and querying BAM files. */
+public class BAMFileReader2 extends SamReader.ReaderImplementation {


This looks like a huge class to be used only one method (BAMFileReader2.findVirtualOffsetOfFirstRecord(seekableStream) in in BAMSBIIndexer.findVirtualOffsetOfFirstRecord

It is, however it's a temporary workaround until the upstream PR is available.

magicDGS · 2018-07-19T17:13:21Z

src/main/java/htsjdk/samtools/SBIIndex.java

+ * SBI is an index into BGZF-compressed data files, which has an entry for the file position of the
+ * start of every <i>n</i>th record. Reads files that were created by {@link BAMSBIIndexer}.
+ */
+public final class SBIIndex implements Serializable {


If this will be in htsjdk, add a TODO pointing to the PR to be removed once htsjdk is upgraded with that change.

Good idea - I'll do that.

magicDGS · 2018-07-19T17:13:42Z

src/main/java/htsjdk/samtools/SBIIndexWriter.java

+ * in an index every <i>n</i>th record. When there are no records left call {@link #finish} to
+ * complete writing the index.
+ */
+public final class SBIIndexWriter {


If this will be in htsjdk, add a TODO pointing to the PR to be removed once htsjdk is upgraded with that change.

magicDGS · 2018-07-19T17:15:49Z

src/main/java/org/disq_bio/disq/HtsjdkReadsRdd.java

+ *
+ * @see HtsjdkReadsRddStorage
+ */
+public class HtsjdkReadsRdd {


Why does it requires the prefix Htsjdk (here and other code in this package)? If we said that this library targets to use the htsjdk library interfaces (in the future, I hope that the proper interfaces in htsjdk-next), we do not require this prefix.

I added it because of a discussion we had a while back about using different data models (i.e. non-htsjdk) for reads and variants. Interested to see what others think too.

If changed to RecordsRdd<Metadata, Record>, the implementation can either maintain the prefix or be called SAMRecordRdd.

I'd prefer to see the Htsjdk prefixes not be in the top level of the API as well. The metadata and record types can be generic as discussed here and in some other comments. Not that it should block the initial commit, but I do like the idea. Also, is it worth abstracting RDD to a type parameter at top level to ease migration to DataSet ? It would help with unifying interface.

Please keep the Htsjdk prefixes for now.

magicDGS · 2018-07-19T17:17:18Z

src/main/java/org/disq_bio/disq/HtsjdkReadsRdd.java

+public class HtsjdkReadsRdd {
+
+  private final SAMFileHeader header;
+  private final JavaRDD<SAMRecord> reads;


Are the reads header-less? If so, add to the top-class javadoc.

I added some more javadoc here.

magicDGS · 2018-07-19T17:34:13Z

src/main/java/org/disq_bio/disq/HtsjdkVariantsRdd.java

+ *
+ * @see HtsjdkVariantsRddStorage
+ */
+public class HtsjdkVariantsRdd {


This could be a common interface for reads/variants and any other kind of records (it will be nice to have FASTQ support eventually and other tribble features). What's about using a RecordsRdd<Metadata, Record>? For reads, it will be RecordsRdd<SAMFileHeader, SAMRecord> and for variants RecordsRdd<VCFHeader, VariantContext> - that is more extensible for other formats (and for moving to a different library, or htsjdk-next, in the future).

Yes, that's a good idea.

This probably needs more discussion. Should HtsjdkReadsRdd still exist and implement the interface? Should its only methods be getMetadata() and getRecords(), or should it also have getHeader() and getReads()? I'd be open to evolving this later - I think it's OK for the API to be unstable for a while.

I think that by now the best option is to keep HtsjdkReadsRdd extending the interface and with the more explicit methods (getHeader/getReads) and then decide if it is really required to have the implementation. This is in line with some discussion that we had in htsjdk-next-beta, where we are planning to add a high-level abstraction for every record reader/iterator with common methods. Maybe at some point disq can extend directly that interfaces to make, when possible, processing on spark/hadoop more similar to processing on a local machine.

Ideally I would like to make distributed processing as similar as possible to local processing, although that's not always possible. Do you have a link to the discussion in htsjdk-next-beta?

We are discussing this design of readers in slack
@lbergelson, you were the one proposing a RecordIterator<Record, Metadata> and RecordReader<Record, Metadata, QueryKey>
@tomwhite - if you want, I can ask the rest of maintainers to add you to the slack chat (I think that it will be fine)

I like the idea of having both a super interface and also a specialized class with convenience methods for reads.

magicDGS · 2018-07-19T17:34:52Z

src/main/java/org/disq_bio/disq/HtsjdkVariantsRddStorage.java

+public class HtsjdkVariantsRddStorage {
+
+  /** An option for configuring how to write a {@link HtsjdkVariantsRdd}. */
+  public interface WriteOption {}


Same as before - extract as VariantsWriteOption instead

magicDGS · 2018-07-19T17:37:30Z

src/main/java/org/disq_bio/disq/HtsjdkVariantsRddStorage.java

+  }
+
+  /** An option for configuring the number of files to write a {@link HtsjdkVariantsRdd} as. */
+  public enum FileCardinalityWriteOption implements WriteOption {


This could be shared by reads/variants/other - maybe have a common WriteOption for all kind of writing and for the format include a different class FormatOption (extending or not WriteOption).

magicDGS · 2018-07-19T17:37:41Z

src/main/java/org/disq_bio/disq/HtsjdkVariantsRddStorage.java

+   * An option for controlling which directory to write temporary part files to when writing a
+   * {@link HtsjdkVariantsRdd} as a single file.
+   */
+  public static class TempPartsDirectoryWriteOption implements HtsjdkReadsRddStorage.WriteOption {


Same as cardinality option

magicDGS · 2018-07-19T17:39:10Z

src/main/java/org/disq_bio/disq/HtsjdkVariantsRddStorage.java

+import org.disq_bio.disq.impl.formats.vcf.VcfSource;
+
+/** The entry point for reading or writing a {@link HtsjdkVariantsRdd}. */
+public class HtsjdkVariantsRddStorage {


There is a lot of paralelism in this class with the reads counterpart - maybe an abstract class would keep in sync both (I saw that there is no NIO support in variants yet, but it could throw by now).

That makes sense too.

Looking again, there's some commonality, but I'm not sure how useful it is to have an abstract base class at this point. Would it be a distraction to the user looking at javadoc if some methods are in a base class?

I believe that the javadoc can show the base class methods, and the javadoc can always be overriden even if the implementation is to call the super method.

tomwhite · 2018-07-26T10:54:59Z

Thanks for all the feedback @magicDGS. I've gone through them and made changes in a new commit.

magicDGS

Here is a proposal for the enums, as I think that I did not explain how I was imagining it.

magicDGS · 2018-07-27T07:16:32Z

src/main/java/org/disq_bio/disq/HtsjdkReadsRddStorage.java


 /** The entry point for reading or writing a {@link HtsjdkReadsRdd}. */
 public class HtsjdkReadsRddStorage {

  /** An option for configuring how to write a {@link HtsjdkReadsRdd}. */
-  public interface WriteOption {}
+  public interface WriteOption {
+    static AbstractSamSink getSink(


I thougth this more like a method for the FormatWriteOption instead of here, and an non-static method (see the proposal below).

magicDGS · 2018-07-27T07:22:27Z

src/main/java/org/disq_bio/disq/HtsjdkReadsRddStorage.java

+              "Unrecognized cardinality: " + fileCardinalityWriteOption);
+      }
+    }
+  }

  /** An option for configuring which format to write a {@link HtsjdkReadsRdd} as. */
  public enum FormatWriteOption implements WriteOption {


This could be implemented as (I skipped some stuff):

public enum FormatWriteOption implements WriteOption { BAM(() -> new BamSink(), SamFormat.BAM) ... // initilalized in constructor private final Supplier<AbstractSamSink> supplier; private final SamFormat format; ... public AbstractSamSink getSink() { return supplier.get(); } public SamFormat getSamFormat() { return format; } }

magicDGS · 2018-07-27T07:28:15Z

src/main/java/org/disq_bio/disq/HtsjdkReadsRddStorage.java

  }

  /** An option for configuring the number of files to write a {@link HtsjdkReadsRdd} as. */
  public enum FileCardinalityWriteOption implements WriteOption {
    /** Write a single file specified by the path. */
    SINGLE,
    /** Write multiple files in a directory specified by the path. */
-    MULTIPLE
+    MULTIPLE;


Here the same, it could be refactored to be:

public enum FileCardinalityWriteOption implements WriteOption { SINGLE(opt -> opt.getSink()), MULTIPLE(opt -> new AnySamSinkMultiple(opt.getSamFormat())); // initialized on construction private Function<FormatWriteOption, AbstractSamSink> sinkProvider; ... public AbstractSamSink getSink(final FormatWriteOption option) { return sinkProvider.apply(option); } }

magicDGS · 2018-07-27T07:32:00Z

src/main/java/org/disq_bio/disq/impl/formats/sam/SamFormat.java

@@ -54,4 +57,17 @@ public static SamFormat fromPath(String path) {
    }
    return null;
  }
+
+  public AbstractSamSource createAbstractSamSource(FileSystemWrapper fileSystemWrapper) {
+    switch (this) {


The same as before: instead of a switch with this, the way will be to have a Function<FileSystemWrapper, AbstractSamSource> instead, which makes it cleaner (and does not require checking for equality).

magicDGS · 2018-07-27T07:34:00Z

src/main/java/org/disq_bio/disq/HtsjdkVariantsRddStorage.java

@@ -155,37 +178,12 @@ public void write(HtsjdkVariantsRdd htsjdkVariantsRdd, String path, WriteOption.
      tempPartsDirectory = path + ".parts";
    }

-    getSink(formatWriteOption, fileCardinalityWriteOption)
+    WriteOption.getSink(formatWriteOption, fileCardinalityWriteOption)


In the case that the refactoring is done, this line will read: fileCardinalityWriteOption.getSink(formatWriteOption), which is cleaner from my point of view. And it could be understood as: creates a sink with a concrete file cardinality using the format requested.

magicDGS · 2018-07-27T07:40:38Z

I also think that storage classes and related enums/options should not be referenced in the other classes (e.g. from* methods in SamFormat). That's why I propose to have the suppliers in the enums instead, and now I understand why separating the PR into several bits was a bit difficult.

Should we maybe discuss somewhere (Slack?) the design and try to deconvolute the classes to make the format more independet from the storage classes?

tomwhite · 2018-07-27T10:29:06Z

@magicDGS I've moved the WriteOptions interface up to the top-level as you suggested, and removed the switch statements. Please let me know what you think.

tomwhite · 2018-07-31T15:10:57Z

@lbergelson you might want to have a look at this too.

magicDGS · 2018-08-03T08:53:41Z

@tomwhite - I did a quick pass over it and it looks good for me. Maybe the SamFormat and ReadsFormatWriteOption converters should be linked in the reverse direction, but that could wait and/or we can discuss that later as it evolves the code once it is in.

I am planning to review whenever I have a bit more of time the implementation package.

tomwhite · 2018-08-06T15:30:21Z

@magicDGS Thanks for taking another look.

Has anyone else got any comments before we commit this?

jamesemery · 2018-08-15T16:32:48Z

src/main/java/org/disq_bio/disq/HtsjdkReadsRddStorage.java

+    String tempPartsDirectory = null;
+    if (tempPartsDirectoryWriteOption != null) {
+      tempPartsDirectory = tempPartsDirectoryWriteOption.getTempPartsDirectory();
+    } else if (fileCardinalityWriteOption == FileCardinalityWriteOption.SINGLE) {


There was a PR in hadoop bam for independently specifying where the temp parts directory lives. Not being able to configure it to something other than outputPath.parts can be restricting with large files on restricted disks.

You can do this in Disq by setting a TempPartsDirectoryWriteOption.

jamesemery · 2018-08-15T17:09:42Z

src/main/java/org/disq_bio/disq/impl/formats/cram/CramSource.java

+    SerializableHadoopConfiguration confSer =
+        new SerializableHadoopConfiguration(jsc.hadoopConfiguration());
+
+    return pathSplitSource


Clarifying question, when you split crams into partitions, they are being split so each cram container is held in its entirety within a partition? Does what happens in degenerate cases with very large cram containers that number fewer than the target number of partitions for splitting?

CRAM containers are not split, so each one is read entirely by exactly one partition. In the case of a container that is bigger than the file split size some partitions will be empty.

cmnbroad

I tried to stay pretty high level. A few minor comments/preferences inline, and:

I'd propose the use of "disq" as a prefix at the top level. So disqReadsRDD and disqReadsRDDStorage, or in my generified world (see inline comments), disqReads, disqReadsStorage.
I'd prefer gradle and TestNG over maven and Junit
Whats the state of 2bit ref support ?
Would be good to get a couple of hg38 test files in early on
Are we going to support BCF ?

cmnbroad · 2018-08-16T13:00:16Z

src/main/java/org/disq_bio/disq/HtsjdkReadsRdd.java

+ *
+ * @see HtsjdkReadsRddStorage
+ */
+public class HtsjdkReadsRdd {


I'd prefer to see the Htsjdk prefixes not be in the top level of the API as well. The metadata and record types can be generic as discussed here and in some other comments. Not that it should block the initial commit, but I do like the idea. Also, is it worth abstracting RDD to a type parameter at top level to ease migration to DataSet ? It would help with unifying interface.

cmnbroad · 2018-08-16T13:23:46Z

src/main/java/org/disq_bio/disq/HtsjdkReadsRddStorage.java

+   *     ReadsFormatWriteOption} and {@link FileCardinalityWriteOption}
+   * @throws IOException if an IO error occurs while writing
+   */
+  public void write(HtsjdkReadsRdd htsjdkReadsRdd, String path, WriteOption... writeOptions)


I know the Java APIs uses varargs like this in a few places, but I always find it awkward especially in library APIs. Except in a a few rare cases like String formatting. Perhaps I'm the only one.

Thanks for the comments @cmnbroad. I've responded to them below. I don't think any of them should hold up committing this as they can all be addressed separately.

I'd propose the use of "disq" as a prefix at the top level.

So that would be DisqReads etc? That would duplicate the project/package name in the class, which is a bit redundant. I looked at generifying briefly but didn't see a huge gain. We can revisit that though.

I'd prefer gradle and TestNG over maven and Junit

I'm more familiar with Maven and JUnit, but I don't have an issue with the others.

Whats the state of 2bit ref support ?

It's not supported. Ideally we'd have 2bit support in htsjdk, then we can use it here.

Would be good to get a couple of hg38 test files in early on

Agreed

Are we going to support BCF?

I wasn't planning to, unless there's a lot of demand for it.

I know the Java APIs uses varargs like this in a few places, but I always find it awkward especially in library APIs.

Do you have a preferred alternative?

tomwhite · 2018-08-28T13:51:42Z

This code passes the Travis build now. I've also opened #19 to address one of the issues identified by @magicDGS and @cmnbroad (we can open more if there are any more comments, e.g. from @lbergelson).

I'd like to merge this in the next day or so unless there are any objections.

droazen · 2018-08-28T16:24:58Z

@tomwhite As discussed in person, my main comment on this PR is on the lack of checked-in realistic test data. I'd like to see some medium-sized (~a couple hundred MB) bams and crams added, and tests run continuously on them. However, given that some decisions will have to be made first on how to host and version this large test data (git lfs? something else?), I'd be satisfied for now if you created a separate ticket for this task and addressed it in a future PR. Otherwise, 👍 from me.

heuermh

Made a review pass through the public APIs.

pom.xml

heuermh · 2018-08-28T16:02:00Z

pom.xml

+        <maven.compiler.source>1.8</maven.compiler.source>
+    </properties>
+
+    <dependencies>


Dependency versions are better specified in a separate <dependencyManagement> section. I also like to define version properties so that dependencies from the same groupId with the same version can be updated in one place.

Is a <dependencyManagement> section useful if there are no child POMs? For the version numbers I've pulled out version properties for htsjdk, Hadoop, and Spark.

heuermh · 2018-08-28T16:05:06Z

pom.xml

+        </dependency>
+        <dependency>
+            <groupId>org.apache.spark</groupId>
+            <artifactId>spark-core_2.11</artifactId>


Spark support for Scala version 2.12 is coming soon, and there is a chance for backwards-incompatible changes in the Spark 2.x series. Do we need to consider releasing for a matrix of Spark & Spark+Scala versions?

Yes, that's a good idea. It would simplify our lives if we can avoid using Spark features that have backwards-incompatible changes.

heuermh · 2018-08-28T16:07:06Z

pom.xml

+        <dependency>
+            <groupId>org.apache.spark</groupId>
+            <artifactId>spark-core_2.11</artifactId>
+            <version>2.0.2</version>


Spark version 2.4.0 is in RC phase; 2.0.x is rather old at this point. What should our policy be for keeping up with Spark releases?

Yes, that is pretty old - I've updated to 2.2.2.

Spark and Hadoop are provided dependencies so it's possible to run against later versions of course. My thinking was to use slightly older versions of Spark and Hadoop to allow for a range, however it would be good to test various versions.

src/main/java/org/disq_bio/disq/FileCardinalityWriteOption.java

heuermh · 2018-08-28T16:17:44Z

src/main/java/org/disq_bio/disq/HtsjdkReadsRddStorage.java

+   * @param sparkContext the Spark context to use
+   * @return a {@link HtsjdkReadsRddStorage}
+   */
+  public static HtsjdkReadsRddStorage makeDefault(JavaSparkContext sparkContext) {


Similar thing here, perhaps public API methods should accept SparkContext instead of JavaSparkContext.

heuermh · 2018-08-28T16:18:47Z

src/main/java/org/disq_bio/disq/HtsjdkReadsRddStorage.java

+  }
+
+  /**
+   * @param splitSize the requested size of file splits when reading


What are the units for splitSize, number of records?

Bytes. Updated doc comment.

src/main/java/org/disq_bio/disq/ReadsFormatWriteOption.java

src/main/java/org/disq_bio/disq/TempPartsDirectoryWriteOption.java

heuermh · 2018-08-28T16:26:47Z

src/main/java/org/disq_bio/disq/impl/file/Merger.java

+                    !(FilenameUtils.getBaseName(f).startsWith(".")
+                        || FilenameUtils.getBaseName(f).startsWith("_")))
+            .collect(Collectors.toList());
+    fileSystemWrapper.concat(conf, filteredParts, outputFile);


As far as I know, concat fails on encrypted HDFS file systems. We have a workaround in ADAM, mentioned previously.

Ah good to know. There's a fallback in HadoopFileSystemWrapper.

tomwhite · 2018-08-30T09:41:37Z

@heuermh thanks for the review! I've responded to your comments - please let me know what you think.

droazen · 2018-08-31T15:28:39Z

Opened #23 to capture my review comment.

heuermh · 2018-08-31T16:53:02Z

@tomwhite I found it odd that this pull request was from a branch on the disq-bio/disq repo instead of one on your fork. I guess there isn't anything wrong with that, just not what I'm used to in other Github based projects.

Perhaps now would be a good time to discuss git workflow (issue #11) and related topics?

Specifically, I'm wondering if we should squash commits in this pull request and use the Rebase and merge button (which is what we do in BDG projects, to keep commit history clean) or use the Squash and merge button (which lists individual commit description lines in the merge commit message) or merge as-is, retaining all the intermediate commits.

droazen · 2018-08-31T17:24:49Z

I'd vote for "Squash and merge" -- it's possible to edit the final squashed commit message from the github UI when using that option, and it doesn't require the PR author to manually squash as a final step.

lbergelson

@tomwhite I have some comments. They're a bit randomly distributed since I didn't get through the whole PR.

lbergelson · 2018-08-31T17:21:22Z

src/main/java/htsjdk/samtools/BAMFileReader2.java

+   * @param inflaterFactory InflaterFactory used by BlockCompressedInputStream
+   * @throws IOException
+   */
+  BAMFileReader2(


We should do an htsjdk release soon...

lbergelson · 2018-08-31T17:24:10Z

src/main/java/org/disq_bio/disq/FileCardinalityWriteOption.java

+import org.disq_bio.disq.impl.formats.vcf.VcfSinkMultiple;
+
+/** An option for configuring whether to write output in a single file, or multiple files. */
+public enum FileCardinalityWriteOption implements WriteOption {


FileCardinalityWriteOption seems more confusing and hard to discover than it needs to be. Maybe FileSplittingWriteOption would be clearer?

The word "splitting" is a bit overloaded with (Hadoop) file splits. Maybe FileNumberWriteOption?

lbergelson · 2018-08-31T17:27:37Z

src/main/java/org/disq_bio/disq/FileCardinalityWriteOption.java

+
+  FileCardinalityWriteOption(
+      Function<ReadsFormatWriteOption, AbstractSamSink> samSinkProvider,
+      Function<VariantsFormatWriteOption, AbstractVcfSink> vcfSinkProvider) {


Its seems a bit weird to have the enum know about the writers vs the writers knowing which options they support. Maybe this could be solved if the bam/vcf writers shared some common ancestor?

Yes, it would be nice to invert this. Note that it doesn't leak into the public API at all, so not an issue from that point of view. (#26)

lbergelson · 2018-08-31T19:40:11Z

src/main/java/org/disq_bio/disq/HtsjdkVariantsRdd.java

+ *
+ * @see HtsjdkVariantsRddStorage
+ */
+public class HtsjdkVariantsRdd {


I like the idea of having both a super interface and also a specialized class with convenience methods for reads.

lbergelson · 2018-08-31T19:43:30Z

src/main/java/org/disq_bio/disq/impl/file/FileSystemWrapper.java

+ */
+public interface FileSystemWrapper extends Serializable {
+
+  boolean usesNio();


I think documenting this interface more completely would be useful, there are a lot of details to each of these operations and it's not clear what it relies on.

I.e. how does delete handle directories, what does normalize guarantee, what order does listDirectory specify.

I've added javadoc for all the methods in this class.

lbergelson · 2018-08-31T20:23:16Z

src/main/java/org/disq_bio/disq/FileCardinalityWriteOption.java

+      variantsFormatWriteOption ->
+          new VcfSinkMultiple(VcfFormat.fromFormatWriteOption(variantsFormatWriteOption)));
+
+  private final transient Function<ReadsFormatWriteOption, AbstractSamSink> samSinkProvider;


Why do these need to be transient? It seems weird.

Yes, it does indicate that it's not right. Opened #26

lbergelson · 2018-08-31T20:35:35Z

src/main/java/org/disq_bio/disq/impl/file/SeekableByteChannelPrefetcher.java

+ * computation and communication for you. (Of course this is only worthwhile if the underlying
+ * SeekableByteChannel doesn't already implement prefetching).
+ */
+public final class SeekableByteChannelPrefetcher implements SeekableByteChannel {


This can be replaced now that we've resolved broadinstitute/gatk#3500!

Yes, we can.

lbergelson · 2018-08-31T20:49:19Z

README.md

+### Ordering Guarantees
+
+This library does not do any sorting, so it is up to the user to understand what is being read or written. Furthermore,
+no checks are carried out to ensure that the records being read or written are consistent with the header. E.g. it


Maybe we should consider adding a check like this. It tends to find bugs.

We could add it as an option (default off).

lbergelson · 2018-08-31T20:51:45Z

src/main/java/org/disq_bio/disq/impl/formats/sam/SamFormat.java

+  CRAM(".cram", ".crai", CramSource::new),
+  SAM(".sam", null, fileSystemWrapper -> new SamSource());
+
+  private String extension;


I'd make these final, mutable anything in an enum is always weird.

lbergelson · 2018-08-31T20:52:10Z

src/main/java/org/disq_bio/disq/impl/formats/sam/SamFormat.java

+
+  SamFormat(
+      String extension,
+      String indexExtension,


Will there be problems when we support multiple index types for a formate? I.e. bam.csi.

Yes, this may need changing at that point.

tomwhite · 2018-09-04T07:27:26Z

I found it odd that this pull request was from a branch on the disq-bio/disq repo instead of one on your fork

I agree it's probably more conventional for PRs to come from the contributor's fork - I'll do that in future.

In terms of workflow, I'm fine with "Squash and merge".

heuermh · 2018-09-04T16:17:08Z

Woot! Thank you for the initial contribution, @tomwhite, and to everyone for review.

lbergelson · 2018-09-04T16:21:06Z

Yay!

tomwhite · 2018-09-04T16:26:02Z

Yes - many thanks for all the reviews!

Initial Disq code contribution.

40d50dc

Corresponds to tomwhite@d3d76ec.

tomwhite mentioned this pull request Jul 19, 2018

Identify initial code contributions #10

Closed

heuermh changed the title ~~Initial Disq code contribution.~~ [DISQ-10] Initial Disq code contribution. Jul 19, 2018

magicDGS reviewed Jul 19, 2018

View reviewed changes

Review feedback

2bf5b6d

tomwhite mentioned this pull request Jul 26, 2018

[DISQ-16] Adding Travis CI configuration. #17

Merged

magicDGS reviewed Jul 27, 2018

View reviewed changes

Top-level WriteOption

1d86cf3

lbergelson self-requested a review August 13, 2018 15:45

jamesemery reviewed Aug 15, 2018

View reviewed changes

cmnbroad reviewed Aug 16, 2018

View reviewed changes

Move to the official release of google-cloud-java

20b1b7b

tomwhite mentioned this pull request Aug 28, 2018

Create common interface for Htsjdk{Reads|Variants}Rdd and Htsjdk{Reads|Variants}RddStorage #19

Closed

heuermh self-requested a review August 28, 2018 15:53

droazen approved these changes Aug 28, 2018

View reviewed changes

heuermh requested changes Aug 28, 2018

View reviewed changes

Review feedback from heuermh

58afe50

heuermh mentioned this pull request Aug 31, 2018

Design public API for access from multiple languages (Java, Scala, Python, R) #24

Open

heuermh approved these changes Aug 31, 2018

View reviewed changes

lbergelson reviewed Aug 31, 2018

View reviewed changes

Review feedback from lbergelson

3a40dac

tomwhite merged commit 11938a5 into master Sep 4, 2018

tomwhite deleted the tw_disq_initial branch September 4, 2018 16:29

magicDGS mentioned this pull request Sep 9, 2018

Select git workflow for development #11

Closed

heuermh added this to the 0.1.0 milestone Oct 24, 2018

[DISQ-10] Initial Disq code contribution. #14

[DISQ-10] Initial Disq code contribution. #14

Conversation

tomwhite commented Jul 19, 2018

heuermh commented Jul 19, 2018

magicDGS 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

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

tomwhite commented Jul 26, 2018

magicDGS 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

magicDGS commented Jul 27, 2018

tomwhite commented Jul 27, 2018

tomwhite commented Jul 31, 2018

magicDGS commented Aug 3, 2018

tomwhite commented Aug 6, 2018

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

cmnbroad 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

tomwhite commented Aug 28, 2018

droazen commented Aug 28, 2018

heuermh 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

tomwhite commented Aug 30, 2018

droazen commented Aug 31, 2018

heuermh commented Aug 31, 2018

droazen commented Aug 31, 2018

lbergelson 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

cmnbroad left a comment •

edited

Loading