Skip to content

Traverse class ancestors looking for rules to apply in JSON Schema output - #1805

Merged
pkalita-lbl merged 1 commit into
mainfrom
issue-1802-inherit-rules
Jan 4, 2024
Merged

pkalita-lbl merged 1 commit into
mainfrom
issue-1802-inherit-rules

Conversation

@pkalita-lbl

@pkalita-lbl pkalita-lbl commented Dec 22, 2023 •

Copy link
Copy Markdown
Contributor

These changes might warrant a bit of discussion.

You can see a use case described in #1802. In that very simple scenario I think it seems intuitive that MySubClass would inherit the rules defined on MyClass. However it becomes murky if both MySubClass and MyClass define rules. Should it be additive and all rules from both levels of the hierarchy apply? Should the subclass rules replace the superclass rules? Should a subclass be allowed to remove the rules of a superclass without adding any of its own? Or maybe all of this is going against the metamodel because rules is not marked with inherited: true in the first place?

Separately these changes also clean up the way value_presence is translated into JSON Schema. Previously the generator emitted enum: [] when it encountered value_presence: ABSENT. It also emitted this when encountering value_presence: PRESENT:

"properties": {
  "my_slot": {
     "required": true
  }
}

Both of these are non-standard JSON Schema (enum should not be empty and required must be an array of strings), and a strict JSON Schema validator would not accept it. Admittedly, making fields required is one of the most annoying things about JSON Schema because it doesn't follow the same patterns as other keywords (like minimum or pattern, for example). It also makes it really awkward to express "a value is not permitted" (a la value_presence: ABSENT). As far as I can tell the most standard way to do it looks like this:

{
  "properties": {
    "value_must_be_present": {
      "type": "string"
    },
    "value_must_be_absent": {
      "type": "string"
    }
  },
  "required": [
    "value_must_be_present",
  ],
  "not": {
    "required": ["value_must_be_absent"]
  },
  "title": "test",
  "type": "object"
}

The "required": ["value_must_be_absent"] subschema evaluates to True if the value_must_be_absent field has any value and wrapping that in a not keyword inverts that. So it really is saying "no value is allowed" despite looking like it's saying it's simply not required. Again, required in JSON Schema is extremely unintuitive.

In fleshing out some of those required keyword changes I tried adding some additional test cases to an existing test and realized we're not handling this one: #1803. That one could be a bit icky to figure out.

I didn't realize at the outset that I was going to be making two somewhat unrelated sets of changes in this branch. If it makes sense to split out the rules stuff from the value_presence stuff I can do that.

Fixes #1802

…tput

These changes also clean up the way value_presence was being handled. It used to produce the non-standard construct `enum: []` to prohibit any value. This changes it to use an inverted required keyword.
@pkalita-lbl
pkalita-lbl requested a review from cmungall December 22, 2023 20:17
@codecov

codecov Bot commented Dec 22, 2023 •

Copy link
Copy Markdown

Codecov Report

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

Comparison is base (b38b674) 79.76% compared to head (472b363) 79.76%.

Files Patch % Lines
linkml/generators/jsonschemagen.py 75.00% 6 Missing and 5 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1805   +/-   ##
=======================================
  Coverage   79.76%   79.76%           
=======================================
  Files          99       99           
  Lines       11370    11374    +4     
  Branches     2934     2935    +1     
=======================================
+ Hits         9069     9073    +4     
+ Misses       1751     1750    -1     
- Partials      550      551    +1     

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inherited rules do not appear in JSON Schema generator output

2 participants