Autogenerated CWL: version field in wrong place

mr-c commented 5 years ago

Great to see all the CWL integration!

One tweak for the autogenerated CWL: I think you're putting the version of the tool release in a top level filed named version, yes?

https://github.com/CBICA/CaPTk/blob/1987d9933a3e30da9bf53a6661185fab16ea2e3d/data/cwlFiles/BreastTexturePipeline.cwl#L3

If so, that isn't a valid field in a CWL document, and we do have a way to express the version of the software that the description is written for (and to provide identifiers for the CaPTk itself)

Try this:


hints:
  SoftwareRequirement:
     packages:
        CaPTk:
           version: [ 1.7.2 ]
           specs:
             - https://github.com/CBICA/CaPTk
             - https://www.cbica.upenn.edu/captk
             - https://doi.org/10.1117/1.JMI.5.1.011018

sarthakpati commented 5 years ago

Thanks for reporting this, @mr-c! We will look into this.

mr-c commented 5 years ago

I tried to find an RRID to put in the specs field, but there doesn't appear to be one for CaPTk yet.

sarthakpati commented 5 years ago

I tried to find an RRID to put in the specs field, but there doesn't appear to be one for CaPTk yet.

Thank you for letting me know! I have added it: https://scicrunch.org/resources/about/registry/SCR_017323

sarthakpati commented 5 years ago

So, a couple of points:

Since we are generated CWL files in an automated manner, we cannot include the "specs" portion at the moment (need to connect the bib file or something else, will keep that for later).
CaPTk is the front end and is not necessarily what everyone will use (someone can use BreastTexturePipeline independently, if they so wish). We need to ensure that this is taken into account.
The same piece of code that is used for auto-generation of these CWL files needs to work for any other project that is not connected to CaPTk as well, so unfortunately we cannot identify the "package" as "CaPTk".

With these in mind, how does this look:

hints:
  SoftwareRequirement:
    packages:
      BreastTexturePipeline:
        version: 1.7.3.nonRelease

mr-c commented 5 years ago

@sarthakpati The entry under packages is defined as the name of a software package: Like the name you might put after sudo apt install, conda install, etc..

Is BreastTexturePipeline distributed seperately from CaPTk the software package in this repo?

mr-c commented 5 years ago

Also, the version field needs to be a list of strings:

hints:
  SoftwareRequirement:
    packages:
      BreastTexturePipeline:
        version: [ 1.7.3.nonRelease ]

sarthakpati commented 5 years ago

Took care of the version string:


hints:
  SoftwareRequirement:
    packages:
      BreastTexturePipeline:
        version: "[ 1.7.3.nonRelease ]"
``

sarthakpati commented 5 years ago

@sarthakpati The entry under packages is defined as the name of a software package: Like the name you might put after sudo apt install, conda install, etc..

Is BreastTexturePipeline distributed seperately from CaPTk the software package in this repo?

No, it is not. But, if someone wanted, they could build BreastTexturePipeline independently and use it. Is there any way not to tie the name CaPTk to the CWL files that are getting generated?

mr-c commented 5 years ago

The idea is that either the names under packages or the identifiers under specs are sufficient for a workflow platform to pick out a package or container to match.

It appears that CaPTk is a suite of applications: there is a single source code repository, the individual applications share the same build system, they are all part of the same monolithic source and binary releases, and they have a single version number shared by all components.

While the CaPTk suite isn't packaged yet for Debian or (bio)conda, my guess is that it would like be packaged as a single entity unless the dependencies for each app are quite difference or the size of the individual apps are quite large.

Note that this entire SofttwareRequirements section is under hints, so if someone did build the BreastTexturePipeline separately there would be no conflict.

This situation reminds me of the Seqan C++ library that used to ship many applications, but has been since broken out into separate source code repositories, separate source & binary releases, and separate versions for each component.

So perhaps the C++ argument parsing library that generates the CWL description could be given both the application name and package name, to populate the description more fully.

mr-c commented 5 years ago

Took care of the version string:

hints:
 SoftwareRequirement:
   packages:
     BreastTexturePipeline:
       version: "[ 1.7.3.nonRelease ]"

Close! The quotes need to go inside the brackets, like: version: [ "1.7.3.nonRelease" ] Otherwise the value of the version field is a string with brackets inside, not an array of strings :-)

Maybe your build system could run cwltool --validate against the generated CWL descriptions?

sarthakpati commented 5 years ago

Working on it. The YAML library I am using automatically writes out strings with encapsulated quotes (which is the YAML standard, I believe).

sarthakpati commented 5 years ago

Would this work:

hints:
  SoftwareRequirement:
    packages:
      BreastTexturePipeline:
        version: 
          - 1.7.3.nonRelease

mr-c commented 5 years ago

Yep, that's valid,

hints:
  SoftwareRequirement:
    packages:
      BreastTexturePipeline:
        version: 
          - 1.7.3.nonRelease

or

hints:
  SoftwareRequirement:
    packages:
      BreastTexturePipeline:
        version: [ 1.7.3.nonRelease ]

is all good

sarthakpati commented 5 years ago

Excellent. This has been incorporated. Closing issue.

CBICA / CaPTk

Autogenerated CWL: version field in wrong place #596