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

Fix python file2stringarray #278

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

fmigneault
Copy link
Collaborator

Details

This update changes the internal operation accomplished by file2string_array process since it was attempting to create directly a CWL output of type File[].
To do that, it was actually hijacking the internal file cwl.output.json of cwltool by directly populating the contents of the expected output after process execution rather than letting the engine generate it, but format was invalid and still failing the execution anyway. The actual CWL content definition was defined as File, which did not match with the generated File[] pushed in cwl.output.json.

Multi-output (File[]) is also not yet supported in Weaver (see #25) because OGC API - Processes does not allow output multiplicity under a same output ID.

It as been decided that output should be adjusted to be a single File.
This better matches the original description of the process anyway, because old implementation was not even producing a JSON output file:

Transforms a file input into JSON file containing an array of file references as value.

Changes

  • Modify problematic output location and execution methodology of file2string_array process so it does what
    it actually advertises in its abstract description and doesn't result in error after execution.

Fixes

@fmigneault fmigneault requested review from matprov and dbyrns July 16, 2021 19:54
@github-actions github-actions bot added ci/doc Issue related to documentation of the package ci/tests Tests of the package and features feature/CWL Issue related to CWL support process/builtin Issue related to builtin application processes labels Jul 16, 2021
dbyrns
dbyrns previously approved these changes Jul 19, 2021
Copy link
Contributor

@dbyrns dbyrns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything look good.

tests/functional/test_builtin.py Show resolved Hide resolved
CHANGES.rst Show resolved Hide resolved
matprov
matprov previously approved these changes Jul 19, 2021
@fmigneault fmigneault dismissed stale reviews from matprov and dbyrns via aedffab August 10, 2021 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/doc Issue related to documentation of the package ci/tests Tests of the package and features feature/CWL Issue related to CWL support process/builtin Issue related to builtin application processes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] CWLTool throw error the missing folder for Python dist
3 participants