-
Notifications
You must be signed in to change notification settings - Fork 2
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
fix: change order of JSON Schema to search mapper transformations #32
Conversation
In the JSON Schema to search mapper, the suppression flags need to be addressed first, otherwise the jsonref.replace_refs function may remove them. Signed-off-by: Cesar Berrospi Ramis <75900930+ceberam@users.noreply.github.com>
Updating pydantic from 2.8.2 to 2.9.2 triggers a change in JSON Schema from models: the lists with only 1 element get flatten. Signed-off-by: Cesar Berrospi Ramis <75900930+ceberam@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Let's sync in which order we want to merge.
This PR should be merged first, since the current version of pydantic ( |
@@ -51,7 +51,7 @@ def test_json_schema_to_search_mapper_0(): | |||
index_ref = _load(filename) | |||
|
|||
diff = jsondiff.diff(index_ref, index_def) | |||
print(json.dumps(index_def, indent=2)) | |||
# print(json.dumps(index_def, indent=2)) | |||
print(diff) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we comment out the diff
@@ -51,7 +51,7 @@ def test_json_schema_to_search_mapper_0(): | |||
index_ref = _load(filename) | |||
|
|||
diff = jsondiff.diff(index_ref, index_def) | |||
print(json.dumps(index_def, indent=2)) | |||
# print(json.dumps(index_def, indent=2)) | |||
print(diff) | |||
assert index_def == index_ref |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a proper error message in case this assert fails
assert index_def == index_ref, "assert index_def != index_ref"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case of assertion failure, pytest should already provide enough context information but nevertheless I added a commit with more verbose, including the difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments
Signed-off-by: Cesar Berrospi Ramis <75900930+ceberam@users.noreply.github.com>
5cf6f1f
to
3ee6c90
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
jsonref.replace_refs
function may remove them.2.8.2
to2.9.2
triggers a change in JSON Schema from models: the lists with only 1 element get flattened. The documentation is automatically updated.