-
-
Notifications
You must be signed in to change notification settings - Fork 60
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
metaschema predicates pollute salad document namespace #506
Comments
Another option that might work: remove https://github.com/common-workflow-language/cwl-v1.2/blob/a0f2d38e37ff51721fdeaf993bb2ab474b17246b/Process.yml#L15 and copy in the salad metaschema parts we need, making the one change to the |
The TypeScript version has the same issue: https://runkit.com/zimmera/61e180e8b299c2000851e088 (output expandable at bottom) |
@ZimmerA Thanks. Yeah, it is a schema issue; see common-workflow-language/cwl-v1.2#145 for my proposed fix |
Enums are not intended to be pure strings, they're intended to be qualified URIs, but part of the vocabulary so you're able to use the shortened version. It sounds like it is behaving as designed, but the design is surprising. |
I agree for Schema Salad, but not for CWL. CWL uses enums to create command line interfaces https://www.commonwl.org/v1.2/CommandLineTool.html#CommandLineBinding doesn't say that we only use the short versions for the command line (but in fact So I think in practice the concepts are different. |
@tetron common-workflow-language/cwltool#1591 shows that there is no |
Related: #132 |
Ok, I refreshed my recollection of what
What happens is that for the purposes of validation, it gets the short name of the URI. How to get the short name is described in the schema salad spec: https://www.commonwl.org/v1.2/SchemaSalad.html#Short_names cwl-utils / cwljava should provide a conforming "shortname" function if it doesn't already. The python_codegen itself also assumes enum symbols are URIs, and calls safe_name (which computes the short name).
From my perspective, the correct thing to do is 4
I feel that the proposed solution (making enum symbols unparsed strings) is effectively redefining the semantics of the enum data model which is inappropriate for a 1.2.1 version. It would be better to ensure existing validation and Command Line behavior is properly specified (i.e. derive the short name from the URI and check the string against that) for the purposes of checking the source document. I agree that https://www.commonwl.org/v1.2/CommandLineTool.html#CommandLineBinding doesn't explicitly describe behavior for enums. However, in practice the current behavior is that it accepts short name as a string, and then the command line behavior is identical as for a string. |
Alternative title: CWL enum
symbols
get fully expanded instead of being left as pure stringsSource issue: common-workflow-language/cwljava#70
For example,
CommandInputEnumSchema
https://github.com/common-workflow-language/cwl-v1.2/blob/a0f2d38e37ff51721fdeaf993bb2ab474b17246b/CommandLineTool.yml#L355 extendsInputEnumSchema
https://github.com/common-workflow-language/cwl-v1.2/blob/a0f2d38e37ff51721fdeaf993bb2ab474b17246b/Process.yml#L689 which extendssld:EnumSchema
https://github.com/common-workflow-language/cwl-v1.2/blob/a0f2d38e37ff51721fdeaf993bb2ab474b17246b/salad/schema_salad/metaschema/metaschema_base.yml#L135 which defines thesymbols
field as follows:The
_type: "@id"
andidentity: true
might be fine for the metaschema of Schema Salad, but is inappropriate for a CWL Enum definition, where we expect plain strings.cwltool
ignores this, but codegen readers of CWL (cwljava, cwl-utils) cannot.(all of this is also true for the
Output
and non-Command
versions ofEnumSchema
in CWL)I can't find a way to override this with a
specializeFrom
/To
nor with overlapping field definitions, nor with re-rooting the CWL schema to not derive fromsld:EnumSchema
as the metaschema predicates collide with the CWL schema predicates(even if I use
jsonldPredicate: { _id: "sld:symbols" }
):Fix options:
sld:EnumSchema
and renamesymbols
from the metaschema to be something else (and update all other schema salad documents to use this new name in their definitions)symbols
in the metaschema. I like this, but I don't know how to implement it yettype: enum
s.The text was updated successfully, but these errors were encountered: