JSON Schema output definitions attribute name change
#311
Conversation
sujaypatil96
changed the title
definitions attribute name change
|
Looks like this change is causing a lot of tests to fail. |
There was a problem hiding this comment.
All of these changes look great! However, do note that you need to also update everywhere that #/definitions/{...} is used as a reference name, such as:
linkml/linkml/generators/jsonschemagen.py
Line 54 in 0d94482
This should be changed to #/$defs/{...}, I think. Test it out, and let me know if I'm wrong!
The reason you're running into test failures is because a lot of the LinkML tests have expected output written into the repository -- so when you change the JSON Schema file, you'll also need to change the expected output files. I don't actually know how to run the tests via tox (maybe @cmungall knows?), but I use pipenv run python setup.py test to run all the tests, or pipenv run python setup.py test -s {test suite/case name} to run a particular test -- for example, I can run tests relating to #/definitions/{...} that are currently failing for me by running pipenv run python setup.py test -s 'tests.test_generators.test_jsonschemagen'. You'll need to run pipenv install to set up the Pipenv first.
|
Let's discuss testing on the call on Friday You can also run the tests directly from pycharm. note if you do run all the tests then you will get changes in the output folders tht shouldn't be committed. This all needs documented better: #237 |
|
@gaurav: you're exactly right, the references do need to be changed to After this commit, we can see the output as below: Yup, I realized that a few tests would fail considering it's being tested against the previous expected output which used |
|
If the main branch is failing tests, it might be a good idea create a new branch from main, fix/mark as todo all of those tests, and get them passing again before working on other PRs — that way, we can isolate just the tests affecting this change.
Oh, hey, my first PR comment sent from a plane
…---
Typed on a tiny phone.
On Aug 31, 2021, at 6:23 PM, Sujay Patil ***@***.***> wrote:
@gaurav: you're exactly right, the references do need to be changed to $defs as well.
After this commit, we can see the output as below:
{
"$defs": {
"Employee": {
"additionalProperties": false,
"description": "A person employed by an organization",
"properties": {
"age_in_years": {
"description": "The age of a person if living or age of death if not",
"type": "integer"
},
"aliases": {
"description": "An alternative name",
"items": {
"type": "string"
},
"type": "array"
},
"first_name": {
"description": "The first name of a person",
"type": "string"
},
"id": {
"description": "Unique identifier of a person",
"type": "string"
},
"last_name": {
"type": "string"
}
},
"required": [
"id",
"last_name"
],
"title": "Employee",
"type": "object"
},
"Manager": {
"additionalProperties": false,
"description": "An employee who manages others",
"properties": {
"age_in_years": {
"description": "The age of a person if living or age of death if not",
"type": "integer"
},
"aliases": {
"description": "An alternative name",
"items": {
"type": "string"
},
"type": "array"
},
"first_name": {
"description": "The first name of a person",
"type": "string"
},
"has_employees": {
"items": {
"$ref": "#/$defs/Employee"
},
"type": "array"
},
"id": {
"description": "Unique identifier of a person",
"type": "string"
},
"last_name": {
"type": "string"
}
},
"required": [
"id",
"last_name"
],
"title": "Manager",
"type": "object"
},
"Organization": {
"additionalProperties": false,
"description": "An entity comprised of blah blah",
"properties": {
"has_boss": {
"$ref": "#/$defs/Manager"
},
"id": {
"description": "Unique identifier of a person",
"type": "string"
},
"name": {
"description": "human readable name",
"type": "string"
}
},
"required": [
"id"
],
"title": "Organization",
"type": "object"
}
},
"$id": "http://example.org/sample/organization",
"$schema": "http://json-schema.org/draft-07/schema#",
"additionalProperties": true,
"properties": {},
"title": "organization",
"type": "object"
}
Yup, I realized that a few tests would fail considering it's being tested against the previous expected output which used definitions rather than $defs, but there are quite a few of them that are failing as you can see from this action. I'll try to figure out which ones need to be modified exactly.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
Per @gaurav's recommendation in issue #305, the following PR seeks to change the JSON Schema generator such that it outputs the JSON Schema with the new
$defsattribute rather than using thedefinitionsattribute.I tested the PR on the
examples\organization.yamlschema.Here's what the output looks like: