common-workflow-language / cwltool

Common Workflow Language reference implementation
https://cwltool.readthedocs.io/
Apache License 2.0
326 stars 227 forks source link

--make-template fails to create valid yaml on mixed input types #1539

Open alexiswl opened 2 years ago

alexiswl commented 2 years ago

Expected Behavior

Tell us what should happen

cwltool --make-template should create a valid yaml

Actual Behavior

Tell us what happens instead

An invalid yaml is created when input types are mixed.

Workflow Code

Workflow can be found here

$ cwltool --make-template workflows/dragen-germline-pipeline/3.7.5/dragen-germline-pipeline__3.7.5.cwl

Yields

vc_target_coverage: 0  # type "int" (optional)
vc_target_bed_padding: 0  # type "int" (optional)
vc_target_bed:  # type "File" (optional)
    class: File
    path: a/file/path
vc_roh_blacklist_bed:  # type "File" (optional)
    class: File
    path: a/file/path
vc_remove_all_soft_clips: false  # type "boolean" (optional)
vc_max_reads_per_raw_region: 0  # type "int" (optional)
vc_max_reads_per_active_region: 0  # type "int" (optional)
vc_hard_filter: a_string  # type "string" (optional)
vc_enable_vcf_output: false  # type "boolean" (optional)
vc_enable_roh: false  # type "boolean" (optional)
vc_enable_phasing: false  # type "boolean" (optional)
vc_enable_gatk_acceleration: false  # type "boolean" (optional)
vc_enable_decoy_contigs: false  # type "boolean" (optional)
vc_enable_baf:  # type "File" (optional)
    class: File
    path: a/file/path
vc_decoy_contigs: a_string  # type "string" (optional)
sample_sex: valid_enum_value  # enum; valid values: "file:///c/Users/awluc/OneDrive/GitHub/UMCCR/cwl-ica/workflows/dragen-germline-pipeline/3.7.5/dragen-germline-pipeline__3.7.5.cwl#dragen-germline-pipeline--3.7.5/sample_sex/male", "file:///c/Users/awluc/OneDrive/GitHub/UMCCR/cwl-ica/workflows/dragen-germline-pipeline/3.7.5/dragen-germline-pipeline__3.7.5.cwl#dragen-germline-pipeline--3.7.5/sample_sex/female" (optional)
reference_tar:  # type "File"
    class: File
    path: a/file/path
output_file_prefix: a_string  # type "string"
output_directory: a_string  # type "string"
lic_instance_id_location:  # optional
  -  # type "File"
    class: File path: a/file/path
  - a_string # type "string"
fastq_list_rows:  # array of type "file:///c/Users/awluc/OneDrive/GitHub/UMCCR/cwl-ica/schemas/fastq-list-row/1.0.0/fastq-list-row__1.0.0.yaml#fastq-list-row"
  - file:///c/Users/awluc/OneDrive/GitHub/UMCCR/cwl-ica/schemas/fastq-list-row/1.0.0/fastq-list-row__1.0.0.yaml#fastq-list-row
enable_map_align_output: false  # type "boolean" (optional)
enable_duplicate_marking: false  # type "boolean" (optional)
dedup_min_qual: 0  # type "int" (optional)
dbsnp_annotation:  # type "File" (optional)
    class: File
    path: a/file/path

Note the lic_instance_id_location: which can be of type string File or null: see (https://github.com/umccr/cwl-ica/blob/main/workflows/dragen-germline-pipeline/3.7.5/dragen-germline-pipeline__3.7.5.cwl#L205-L214)

lic_instance_id_location:  # optional
  -  # type "File"
    class: File path: a/file/path
  - a_string # type "string"

Your Environment

mr-c commented 2 years ago

Hello @alexiswl

I agree that the class: File path: a/file/path error should be fixed.

And I see how listing each possible type for a mixed type that is not an array of mixed types is messy. How do you think we should represent this situation?

  1. List the type once for each non-null type (with a comment that there should only be one)
  2. Like number 1, but comment out all presented alternatives.
  3. Something else
alexiswl commented 2 years ago

My main problem was that cwltool --make-template 2>/dev/null | yq eval --tojson '.' - crashed, but don't have a strong preference on how it should be rendered.

I would go with 2 where alternatives are commented out.

So:

lic_instance_id_location:  # optional
  -  # type "File"
    class: File path: a/file/path
  - a_string # type "string"

Becomes something like

lic_instance_id_location:  # optional File | string
  - class: File 
    path: a/file/path
  # - a_string