common-workflow-language / schema_salad

Semantic Annotations for Linked Avro Data
https://www.commonwl.org/v1.2/SchemaSalad.html
Apache License 2.0
72 stars 62 forks source link

Support nested typeDSL #711

Closed tom-tan closed 1 year ago

tom-tan commented 1 year ago

This request is to support the nested typeDSL syntax such as File[][].

Once it is merged, we can close common-workflow-language/cwl-v1.3#10.

Note that this change (unintentionally) introduces nested typeDSL syntax to cwl-v1.0 and cwl-v1.1 in addition to cwl-v1.2.1. It does not violate the current spec because it does not mention whether it must be supported or rejected. However, CWL v1.0 and v1.1 workflows that use File[][] is not portable between the engines.

TODO:

codecov[bot] commented 1 year ago

Codecov Report

Merging #711 (77c2483) into main (d6a4085) will increase coverage by 0.09%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #711      +/-   ##
==========================================
+ Coverage   83.61%   83.71%   +0.09%     
==========================================
  Files          22       22              
  Lines        4492     4507      +15     
  Branches     1240     1244       +4     
==========================================
+ Hits         3756     3773      +17     
+ Misses        470      469       -1     
+ Partials      266      265       -1     
Impacted Files Coverage Δ
schema_salad/schema.py 78.38% <ø> (ø)
schema_salad/codegen.py 94.64% <100.00%> (+0.04%) :arrow_up:
schema_salad/main.py 78.26% <100.00%> (+0.11%) :arrow_up:
schema_salad/python_codegen.py 93.68% <100.00%> (+0.03%) :arrow_up:
schema_salad/ref_resolver.py 88.67% <100.00%> (+0.47%) :arrow_up:
tetron commented 1 year ago

We are accepting enhancements for Schema Salad 1.3, so this is great, and these can then be used in CWL 1.3. Unfortunately, we cannot introduce new features into already-released versions.

tom-tan commented 1 year ago

Now it works in both of runtime and compile (i.e., after code-generation) time! When using the metaschema with saladVersion: v1.3 or the generated parser with saladVersion: v1.3 and later, the nested typeDSL is expanded.

Currently I suppose that saladVersion is optional and that assumes saladVersion: "v1.1" if missing.

tom-tan commented 1 year ago

Now it's ready for review!

tom-tan commented 1 year ago

By the way, can I leave my CWL implementain (schema-salad-d and shaft) as is: that is, expanding nested typeDSL recursively even in SALAD v1.1?

I understand it causes a portability issue but this behavior does not violate the spec according to the current description. Or the spec of SALAD v1.1 and earlier (and CWL v1.2 and earlier) will be changed to reject the nested typeDSL?

tom-tan commented 1 year ago

OK, I will open issues for each codegens.

tom-tan commented 1 year ago

Opened! I skipped C++ codegen because it does not deserialise CWL documents.