Adding to Pair docs #114

Merged
merged 1 commit into from Jun 7, 2017

Conversation

Projects
None yet
5 participants
Contributor

ruchim commented Jun 6, 2017

No description provided.

ruchim referenced this pull request in broadinstitute/wdl4s Jun 6, 2017

Merged

Input Pair objects through inputs json #113

Member

geoffjentry commented Jun 6, 2017 edited by briandmaher

👍

Approved with PullApprove

+Pair values can also be specified within the [workflow inputs JSON](https://github.com/broadinstitute/wdl/blob/develop/SPEC.md#specifying-workflow-inputs-in-json) with a `Left` and `Right` value specified using JSON style syntax. For example, given a workflow `wf_hello` and workflow-level variable `twenty_threes`, it could be declared in the workflow inputs JSON as follows:
+```
+{
+ "wf_hello.twenty_threes": { "Left": 23, "Right": "twenty-three" }
@cjllanwarne

cjllanwarne Jun 7, 2017 edited

Contributor

Are those Left and Right capitalizations correct? I think in the output it's all lowercase?

@ruchim

ruchim Jun 7, 2017

Contributor

Json to Pair coercion isn't sensitive towards capitalization (there's a test case in WDL4S for it) but I didn't realize the outputs were all lowercase, that creates a slight inconsistency. We could make it required lowercase for the future but currently Pair creation accepts "Left" or "left"

Contributor

cjllanwarne commented Jun 7, 2017 edited by briandmaher

Minor nitpick, but otherwise 👍

Approved with PullApprove

Collaborator

sooheelee commented Jun 7, 2017 edited

Given the meaning of indexing in genomics (BWA indexing, BAM indexing, VCF indexing), this section reads confusingly for me. Also, it's unclear how a researchers would want to use this feature.

It would be nice to have one line clarifying indices being discussed have two different meanings (coding and in genomics) and also another line giving an example of how a genomicist would best use this feature, e.g. to keep file indices together with the BAM/VCF file.

Here is Ruchi's example and I think it gets right to my second request.

Pair[File, File] mySample = ("/path/to/sample.bam", "/path/to/sample.bai")
Contributor

ruchim commented Jun 7, 2017

@sooheelee I see--alternatively, I could move the pair member access syntax directly inside of the Pair Literals section.

Collaborator

vdauwera commented Jun 7, 2017

The "indexing" jargon overlap is kind of inevitable -- and I don't think the burden of clarifying should be on every WDL technical doc that uses the term (that way lies some amount of madness). For one, it's a term that people familiar with scripting in bioinformatics should already understand. So the problem is really only for newbs. Obviously we want to make sure we help them navigate this stuff, so I would advocate for writing a doc about terminology for researchers looking at scripting for the first time. And when we write up a description of the Pair object for the user docs proper (WDL website/forum) we can make sure to do so in a way that is unambiguous and well furnished with domain-relevant examples.

@ruchim ruchim merged commit d336f91 into develop Jun 7, 2017

1 check passed

code-review/pullapprove Approved by cjllanwarne, geoffjentry
Details

ruchim deleted the rm_pairs branch Jun 7, 2017

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