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

Changed outputSource refScope from 0 to 1 #1714

Closed
wants to merge 1 commit into from

Conversation

GlassOfWhiskey
Copy link
Contributor

This change allows a corret id reference in the outputSource field of WorkflowOutputParameter objects

@GlassOfWhiskey GlassOfWhiskey force-pushed the output-source-refscope branch from c303ca4 to b5eb57f Compare August 18, 2022 12:33
@GlassOfWhiskey GlassOfWhiskey changed the title Changed outputSource refScope from 0 to 2 Changed outputSource refScope from 0 to 1 Aug 18, 2022
@codecov
Copy link

codecov bot commented Aug 18, 2022

Codecov Report

Merging #1714 (9407590) into main (0e2ced5) will decrease coverage by 12.03%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             main    #1714       +/-   ##
===========================================
- Coverage   66.58%   54.54%   -12.04%     
===========================================
  Files          89       45       -44     
  Lines       15850     7950     -7900     
  Branches     4188     2059     -2129     
===========================================
- Hits        10553     4336     -6217     
+ Misses       4207     3090     -1117     
+ Partials     1090      524      -566     
Impacted Files Coverage Δ
cwltool/provenance_constants.py
cwltool/pack.py
cwltool/udocker.py
cwltool/main.py
cwltool/errors.py
cwltool/cuda.py
cwltool/checker.py
cwltool/argparser.py
cwltool/docker.py
cwltool/mpi.py
... and 34 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@GlassOfWhiskey GlassOfWhiskey force-pushed the output-source-refscope branch from b5eb57f to c1b56b1 Compare August 18, 2022 18:54
@GlassOfWhiskey GlassOfWhiskey changed the title Changed outputSource refScope from 0 to 1 Changed outputSource refScope from 0 to 2 Aug 18, 2022
@GlassOfWhiskey GlassOfWhiskey force-pushed the output-source-refscope branch from c1b56b1 to 9fc86ba Compare August 18, 2022 19:18
@GlassOfWhiskey GlassOfWhiskey changed the title Changed outputSource refScope from 0 to 2 Changed outputSource refScope from 0 to 1 Aug 18, 2022
@GlassOfWhiskey GlassOfWhiskey force-pushed the output-source-refscope branch from 9fc86ba to 2b52520 Compare August 18, 2022 19:45
@kinow kinow mentioned this pull request Aug 18, 2022
@mr-c mr-c marked this pull request as draft August 19, 2022 10:26
This change allows a corret id reference in the outputSource field of
WorkflowOutputParameter objects
@mr-c mr-c force-pushed the output-source-refscope branch from 2b52520 to 9407590 Compare August 20, 2022 09:54
@GlassOfWhiskey
Copy link
Contributor Author

GlassOfWhiskey commented Aug 20, 2022

This PR allows to unify the behaviour of cwltool.load_tool parser and the schema_salad generated parser in cwl-utils. Indeed, consider the simple Workflow example

cwlVersion: v1.2
class: Workflow
id: a
inputs:
  a: int
outputs:
  a:
    type: int
    outputSource: a
steps: []

In this case, with cwltool we have

{
    "class": "Workflow",
    "cwlVersion": "v1.2",
    "id": "file:///home/glassofwhiskey/Projects/streamflow/source/examples/a.cwl#a",
    "inputs": [
        {
            "id": "file:///home/glassofwhiskey/Projects/streamflow/source/examples/a.cwl#a/a",
            "type": "int"
        }
    ],
    "outputs": [
        {
            "id": "file:///home/glassofwhiskey/Projects/streamflow/source/examples/a.cwl#a/a",
            "outputSource": "file:///home/glassofwhiskey/Projects/streamflow/source/examples/a.cwl#a/a",
            "type": "int"
        }
    ],
    "steps": []
}

while with the schema_salad parser we have

{
    "class": "Workflow",
    "cwlVersion": "v1.2",
    "id": "file:///home/glassofwhiskey/Projects/streamflow/source/examples/a.cwl#a",
    "inputs": [
        {
            "id": "file:///home/glassofwhiskey/Projects/streamflow/source/examples/a.cwl#a/a",
            "type": "int"
        }
    ],
    "outputs": [
        {
            "id": "file:///home/glassofwhiskey/Projects/streamflow/source/examples/a.cwl#a/a",
            "outputSource": "file:///home/glassofwhiskey/Projects/streamflow/source/examples/a.cwl#a/a/a",
            "type": "int"
        }
    ],
    "steps": []
}

The cwltool behaviour is correct, while the schema_salad parser puts an additional scope level in the outputSource field, which references an element that does not exist.
Note however that the clash between input and output (and potentially step) ids is present in both cwltool and schema_salad parser, but it is not related to the outputSource scope.

@GlassOfWhiskey GlassOfWhiskey requested review from tetron and mr-c August 20, 2022 10:39
@mr-c
Copy link
Member

mr-c commented Sep 5, 2022

Closing; this will get fixed in CWL v1.2.1

@mr-c mr-c closed this Sep 5, 2022
@mr-c mr-c deleted the output-source-refscope branch September 5, 2022 16:15
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

Successfully merging this pull request may close these issues.

2 participants