Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Deprecating CommandInputEnumSchema#inputBinding #12

Open
tom-tan opened this issue Jun 30, 2023 · 4 comments
Open

[Proposal] Deprecating CommandInputEnumSchema#inputBinding #12

tom-tan opened this issue Jun 30, 2023 · 4 comments

Comments

@tom-tan
Copy link
Member

tom-tan commented Jun 30, 2023

It is available since CWL v1.0 but its usecase can be covered with CommandInputParameter#inputBinding and CommandInputRecordField#inputBinding, I guess.

In addition to that, the current behavior of CommandInputEnumSchema#inputBinding with CommandInputParameter#inputBinding in cwltool seems to be unintended.

Here is an example.

enum-order-root-leaf.cwl:

class: CommandLineTool
cwlVersion: v1.2

baseCommand: python
inputs:
  - id: args.py
    type: File
    default:
      class: File
      location: args.py
    inputBinding:
      position: -1
  - id: inp1
    type:
      type: enum
      symbols: [a, b, c]
      inputBinding:
        position: 4
        prefix: -x
    inputBinding:
      position: 1
      prefix: -p
  - id: inp2
    type:
      type: enum
      symbols: [d, e, f]
      inputBinding:
        position: 3
        prefix: -z
    inputBinding:
      position: 2
      prefix: -q
outputs:
  - id: args
    type:
      type: array
      items: string

hints:
  - class: DockerRequirement
    dockerPull: docker.io/python:3-slim

enum-order-job.json:

{
    "inp1": "a",
    "inp2": "d"
}

I expect that it returns the following in which the value a and d occur only once:

{
    "args": [
        "-p",
        "-x",
        "a",
        "-q",
        "-z",
        "d"
    ]
}

However, here is the actual output of cwltool:

$ cwltool enum-order-root-leaf.cwl enum-order-job.json 
...
{
    "args": [
        "-p",
        "a",
        "-x",
        "a",
        "-q",
        "d",
        "-z",
        "d"
    ]
}

There are three options to fix it:

  • Deprecate CommandInputEnumSchema#inputBinding not to care about such complicated cases
    • I prefer this option because I do not know practical usecases for it
    • It also makes the spec simpler
  • Fix cwltool for nested inputBinding
    • It is possible. However, deprecating this field is easier for us
  • Clarify the spec to support current behavior of cwltool
    • It needs much effort because the current behavior is hard to explain

What do you think about it?

Related: common-workflow-language/common-workflow-language#783

@tom-tan
Copy link
Member Author

tom-tan commented Jul 10, 2023

Note: Once we have a consensus about the deprecation, we can leave some of the cases for common-workflow-language/common-workflow-language#783 as undocumented in CWL v1.2.1.

@mr-c
Copy link
Member

mr-c commented Jul 10, 2023

@tom-tan I agree with your expected results; @tetron and I think that this is a cwltool bug; we should add conformance tests for this.

@tom-tan
Copy link
Member Author

tom-tan commented Jul 11, 2023

we should add conformance tests for this.

Does it mean that we take the second option? (i.e., "Fix cwltool for nested inputBinding" in the above description)

@mr-c
Copy link
Member

mr-c commented Oct 6, 2023

we should add conformance tests for this.

Does it mean that we take the second option? (i.e., "Fix cwltool for nested inputBinding" in the above description)

Yes, the second option

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants