Skip to content
This repository has been archived by the owner on Jun 9, 2020. It is now read-only.

Deprecating cwlgen and migrating to CWL-Utils #27

Closed
illusional opened this issue Aug 8, 2019 · 4 comments · Fixed by #47
Closed

Deprecating cwlgen and migrating to CWL-Utils #27

illusional opened this issue Aug 8, 2019 · 4 comments · Fixed by #47

Comments

@illusional
Copy link
Collaborator

illusional commented Aug 8, 2019

There have been advancements in the schema-salad to generate Python classes that more directly mirror the functionality this project provides. As a consequence, it's probably worth investigating migrating some of the generator tests to the new repo and deprecating this project.

I've used the grammar from common-workflow-language/cwl-utils#5 for these initial tests. From migrating some of the cwlgen tests, I've noticed some differences:

  • Identifiers and derived fields are based on the file path are different (for the tests test_unit_import_workflow):

    • Current cwlgen identifier = 1stWorkflow
    • New cwl-utils identifier = file:///path/to/import_workflow.cwl#1stWorkflow
    • This affects identifier derived fields, including: format, source, run.
  • Some method similarities (cwlgen → cwl-utils):

    • get_dict() → save()
    • parse_cwl(cwlfile)load_document(cwlfile)
    • parse_dict → No super clear analogue, but loaded through _RecordLoader(CommandLineTool) || _UnionLoader((CommandLineToolLoader, ...workflow + other loaders)

Additionally, to avoid conflicting with Python keywords we made some specific keyword changes when creating cwlgen, here's how some at least might be mapped back on the initialiser.

  • tool_id | workflow_id | input_id | etcid`
  • StepInput: inputsin_`

Other little differences (more investigation required):


The big difference is that any issues can be solved by regenerating the file based on the schema-salad definition of CWL, so it should always be right. When I get a chance, and once optional init fields are in schema-salad I'd be interested in migrating a real world project to using it.

FYI @mr-c

@multimeric
Copy link

So we're still waiting for schema_salad to support optional args before cwl-utils is particularly usable?

@mr-c
Copy link
Member

mr-c commented May 4, 2020

@TMiguelT It is usable now, just annoying to have to specify None everywhere. See common-workflow-language/schema_salad#262 for more details

@illusional
Copy link
Collaborator Author

Dear all,

I've fixed the related issues, and now cwl-utils is the recommended approach for generating (and parsing) cwl. It's generated right from the spec so you can have confidence it's right and has support for every version of CWL.

My notes above will guide you on the migration path, but a few notes:

  • The parameters are named with camelCase and not snake_case like we've generally used.
  • Take care if you're migrating to a newer spec, as some classes might have changed names (notably: InputParameter -> WorkflowInputParameter)
  • Don't forget to catch all references of cwlgen, as missing one will cause:
    raise RepresenterError('cannot represent an object: %s' % (data,))
    ruamel.yaml.representer.RepresenterError: cannot represent an object: 
    <cwlgen.common.CommandInputArraySchema object at 0x1100a5780>
  • None of my tests failed because of the List[CommentedMap] == ordereddict, I do use self.assertDictEquals.

I migrated my project Janis, which heavily relied on this library, and it only took a few hours: https://github.com/PMCC-BioinformaticsCore/janis-core/pull/21/files. If you have questions, feel free to tag me in this issue or send me a message. Gitter is the best place for this.


Also, come along this Tuesday to the CWL call where I'll talk about the changes I've made and we'll discuss cwlgen and what will happen to it.

@illusional illusional changed the title Investigate cwl-utils and deprecating this project Deprecating cwlgen and migrating to CWL-Utils Jun 2, 2020
@mr-c mr-c closed this as completed in #47 Jun 9, 2020
@illusional illusional reopened this Jun 9, 2020
@illusional
Copy link
Collaborator Author

I marked this issue for closing, but reopening as a placeholder.

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

Successfully merging a pull request may close this issue.

3 participants