Update for subscope and __eq__ support

[tetron]

Incorporates fixes from common-workflow-language/schema_salad#594

[codecov]

Codecov Report

Merging #167 (53ede44) into main (a56a0ad) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
- Coverage   30.47%   30.46%   -0.01%     
==========================================
  Files          28       28              
  Lines       22585    22584       -1     
  Branches     6156     6156              
==========================================
- Hits         6882     6881       -1     
  Misses      14276    14276              
  Partials     1427     1427              

Impacted Files	Coverage Δ
cwl_utils/expression_refactor.py	80.48% <ø> (ø)
cwl_utils/parser/cwl_v1_0.py	30.62% <ø> (-0.02%)	⬇️
cwl_utils/parser/cwl_v1_1.py	27.77% <100.00%> (ø)
cwl_utils/parser/cwl_v1_2.py	29.79% <100.00%> (ø)

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

[mr-c]

To summarize the changes: all identifiers in embedded Process objects under Workflow.steps[].run will have a different computed value in CWL v1.1+ versus CWL v1.0

In CWL v1.1+, run/ is inserted into the scope of those identifiers.

See also subscope under https://www.commonwl.org/v1.1/SchemaSalad.html#Identifier_resolution

[mr-c]

Since we don't have a common function to load an external process (when run is a relative path or URI), it is an open question about what to do there

[lgtm-com]

This pull request fixes 1 alert when merging 53ede44 into a56a0ad - view on LGTM.com

fixed alerts:

• 1 for Unused import

[GlassOfWhiskey]

I vote for consistent behavior between inline and external sources.
However, I understand this can break some cwltool implementations.

[GlassOfWhiskey]

In my opinion, the main point here is decide if we want to put the logic inside the load_document_by_uri function, or keep on the user the burden to call expand_uri before loading internal files.

The second one is more flexible, but it requires users to be aware of the run introduction in CWL. A third alternative could be the introduction of a new load_step_by_uri or a WELL DOCUMENTED flag is_step into the load_document_by_uri function that implicitly applies the subscope for CWL v1.1+ and does nothing for CWL v1.0. The problem is where to place this function, because it is relative to the parent version and not the child one.

The last alternative is to automatically load external files in steps when parsing, calling the correct parser based on the CWL version. IMO this would be the best options for all cases.

[tetron]

Since we don't have a common function to load an external process (when run is a relative path or URI), it is an open question about what to do there

This doesn't affect identifiers in external processes (you mean other files, right?), it only affects the way identifiers are assigned to objects within processes that are declare inline under 'run'.

cwltool currently has an explicit workaround to introduce 'id' fields to inlined processes on v1.0 documents:

https://github.com/common-workflow-language/cwltool/blob/1d23218ac1c960071ea8d382dadc4f8aa5a2db26/cwltool/load_tool.py#L239

This results in the inlined processes getting a random id.

So the best thing would probably be to retroactively add 'run' subscope to v1.0, at least on the "codegen" branch.

[mr-c]

Since we don't have a common function to load an external process (when run is a relative path or URI), it is an open question about what to do there

This doesn't affect identifiers in external processes (you mean other files, right?), it only affects the way identifiers are assigned to objects within processes that are declare inline under 'run'.

Yeah, I know it doesn't change anything with external processes. But then how should we be loading these files; should we also adjust the baseuri?

[tetron]

Yeah, I know it doesn't change anything with external processes. But then how should we be loading these files; should we also adjust the baseuri?

Scoped ids are always added to the fragment portion of the baseuri, relative references are added to the path portion, and when a new document is loaded, that document's URI is used as the base URI. But codegen doesn't follow 'run' and immediately load sub-documents, that's done by the cwltool Workflow class.

[tetron]

Just to clarify, the semantics of subscope do not affect the value of run it affects the value of id fields in objects that are part of the same document under run.

[mr-c]

Thanks for the information @tetron ; then this works well enough for now