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.

Sign up for GitHub

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

Jump to bottom

Adding map and named union types to Schema SALAD #672

Merged
merged 22 commits into from
Nov 22, 2023
Merged

Adding map and named union types to Schema SALAD #672

merged 22 commits into from
Nov 22, 2023

Conversation

GlassOfWhiskey
Copy link
Contributor

@GlassOfWhiskey GlassOfWhiskey commented Feb 2, 2023 •
edited
Loading

This commit adds the map and named union types to represent arbitrary dictionaries (see https://avro.apache.org/docs/1.11.1/specification/#maps and https://avro.apache.org/docs/1.11.1/specification/#unions).

This will provide a way to model CWL inputs as valid Schema SALAD definitions, solving #276. Indeed, introducing a named union like the following:

name: CWLObjectType
type: union
items:
  - "null"
  - File
  - Directory
  - type: array
    items: CWLObjectType
  - type: map
    values: CWLObjectType
  - Any

it is possible to represent any CWL object, e.g. the default value injectd in an InputParameter. Also, a CWL jobfile can be represented as a map<string, CWLObjectType>.

@codecov
Copy link

codecov bot commented Feb 2, 2023 •
edited
Loading

Codecov Report

Attention: 71 lines in your changes are missing coverage. Please review.

Comparison is base (68cc352) 83.68% compared to head (13ab8d8) 83.62%.

Files Patch % Lines
schema_salad/avro/schema.py 59.30% 21 Missing and 14 partials ⚠️
schema_salad/java_codegen.py 48.48% 15 Missing and 2 partials ⚠️
schema_salad/cpp_codegen.py 87.96% 6 Missing and 7 partials ⚠️
schema_salad/dotnet_codegen.py 94.44% 0 Missing and 1 partial ⚠️
schema_salad/makedoc.py 90.90% 0 Missing and 1 partial ⚠️
schema_salad/python_codegen.py 96.29% 0 Missing and 1 partial ⚠️
schema_salad/schema.py 94.44% 1 Missing ⚠️
schema_salad/sourceline.py 66.66% 1 Missing ⚠️
schema_salad/typescript_codegen.py 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #672      +/-   ##
==========================================
- Coverage   83.68%   83.62%   -0.07%     
==========================================
  Files          22       22              
  Lines        4580     4850     +270     
  Branches     1239     1311      +72     
==========================================
+ Hits         3833     4056     +223     
- Misses        483      516      +33     
- Partials      264      278      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mr-c mr-c marked this pull request as draft February 3, 2023 18:47
@GlassOfWhiskey GlassOfWhiskey changed the title Adding map type to Schema SALAD Adding map and named union types to Schema SALAD Feb 4, 2023
@GlassOfWhiskey GlassOfWhiskey force-pushed the avro-map-schema branch 10 times, most recently from 1ea4307 to 70a2bfa Compare February 7, 2023 08:45
@GlassOfWhiskey GlassOfWhiskey mentioned this pull request Feb 7, 2023
Merged
@GlassOfWhiskey GlassOfWhiskey force-pushed the avro-map-schema branch 8 times, most recently from 9d86f0b to bf96061 Compare February 12, 2023 11:36
@GlassOfWhiskey GlassOfWhiskey marked this pull request as ready for review February 12, 2023 11:36
This was referenced Feb 12, 2023
Merged
Merged
mr-c
mr-c reviewed Feb 12, 2023
View reviewed changes
schema_salad/avro/schema.py Outdated Show resolved Hide resolved
@GlassOfWhiskey
Fixed LoadingOptions class
005be60
@GlassOfWhiskey
Added flatten directive to ArraySchema
a70e5e2
@GlassOfWhiskey
Added NamedMapSchema and NamedUnionSchema
d6ff797
@GlassOfWhiskey
Modernize MyPy type annotations
4cb368d
@GlassOfWhiskey
Limited support for MapSchema
08f1d1c
@GlassOfWhiskey
Restored MapSchema support
1dbfbfc
@GlassOfWhiskey
Document Schema Salad 1.3
b3dd933
@GlassOfWhiskey
Add support for noLinkCheck in codegen
53165ec 
This commit introduces support for the `noLinkCheck` `jsonldPredicate`
in the Schema SALAD codegen toolchain, and in particular in the
`URILoader` stack. When `noLinkCheck` is set to `true`, no link checking
is performed for all the underlying objects hierarchy. The need to
propagate its value to the hierarchy makes it necessary to modify also
the `LoadingOptions` data structure.

All parsers but the Python one never perform link checking up to now, so
this update makes no modification to their actual behaviour. However, it
enables the `URILoader` classes to receive the `noLinkCheck` parameter
for future implementations.
@GlassOfWhiskey
Updated typescript generator
fd106ab
@GlassOfWhiskey
Updated cpp generator
de3e65a
@GlassOfWhiskey
Fixed tests
388f158
@GlassOfWhiskey
Added my name to the list of contributors
11a10ca
@GlassOfWhiskey
Fixed python codegen
04f948e
@GlassOfWhiskey
Fix misinterpretation of absent fields
043b5ca 
This commit removes an erroneous null check, which caused parsers to
evaluate as erroneous some correct documents.
@GlassOfWhiskey GlassOfWhiskey force-pushed the avro-map-schema branch 2 times, most recently from de73e1e to 127fa8a Compare November 21, 2023 23:57
@GlassOfWhiskey GlassOfWhiskey force-pushed the avro-map-schema branch 6 times, most recently from 829ea84 to bc718b6 Compare November 22, 2023 10:38
mr-c
mr-c approved these changes Nov 22, 2023
View reviewed changes
Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

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

Using common-workflow-language/cwl-utils#199 , I ran cwltool through the CWL v1.2 conformance test using --fast-parser and everything passed!

@mr-c mr-c merged commit 1487339 into main Nov 22, 2023
41 of 43 checks passed
@mr-c mr-c deleted the avro-map-schema branch November 22, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mr-c mr-c mr-c approved these changes

@tom-tan tom-tan Awaiting requested review from tom-tan

@ZimmerA ZimmerA Awaiting requested review from ZimmerA

@SGSSGene SGSSGene Awaiting requested review from SGSSGene

@tetron tetron Awaiting requested review from tetron

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@GlassOfWhiskey @tetron @mr-c