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.

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: fix output to JSON schema [WIP] #235

Merged
merged 5 commits into from
Dec 16, 2023
Merged

fix: fix output to JSON schema [WIP] #235

merged 5 commits into from
Dec 16, 2023

Conversation

tlambert03
Copy link
Owner

@tlambert03 tlambert03 commented Dec 15, 2023

This is a WIP, trying to fix OME.model_json_schema() which is broken:

In [1]: OME.model_json_schema()

File ~/miniforge3/envs/ome-types/lib/python3.11/site-packages/pydantic/json_schema.py:2110, in GenerateJsonSchema._build_definitions_remapping(self)
   2107         json_ref = JsonRef(self.ref_template.format(model=defs_ref))
   2108         defs_to_json[defs_ref] = json_ref
-> 2110 return _DefinitionsRemapping.from_prioritized_choices(
   2111     self._prioritized_defsref_choices, defs_to_json, self.definitions
   2112 )

File ~/miniforge3/envs/ome-types/lib/python3.11/site-packages/pydantic/json_schema.py:175, in _DefinitionsRemapping.from_prioritized_choices(prioritized_choices, defs_to_json, definitions)
    172 # Deduplicate the schemas for each alternative; the idea is that we only want to remap to a new DefsRef
    173 # if it introduces no ambiguity, i.e., there is only one distinct schema for that DefsRef.
    174 for defs_ref, schemas in schemas_for_alternatives.items():
--> 175     schemas_for_alternatives[defs_ref] = _deduplicate_schemas(schemas_for_alternatives[defs_ref])
    177 # Build the remapping
    178 defs_remapping: dict[DefsRef, DefsRef] = {}

File ~/miniforge3/envs/ome-types/lib/python3.11/site-packages/pydantic/json_schema.py:2216, in _deduplicate_schemas(schemas)
   2215 def _deduplicate_schemas(schemas: Iterable[JsonDict]) -> list[JsonDict]:
-> 2216     return list({_make_json_hashable(schema): schema for schema in schemas}.values())

File ~/miniforge3/envs/ome-types/lib/python3.11/site-packages/pydantic/json_schema.py:2216, in <dictcomp>(.0)
   2215 def _deduplicate_schemas(schemas: Iterable[JsonDict]) -> list[JsonDict]:
-> 2216     return list({_make_json_hashable(schema): schema for schema in schemas}.values())

TypeError: unhashable type: 'dict'

the reason is our lists of objects StructuredAnnotationsList and ShapesUnion, which use fields that look like this:

Field(
            default_factory=list,
            metadata={  # type: ignore[call-arg]
                "type": "Elements",
                "choices": tuple(
                    {"name": kind.title(), "type": cls} for kind, cls in _KINDS.items()
                ),
            },
        )

the problem is choices...which contains an unhashable dict. That field is really there for xsdata though, which uses it to bind data at runtime. But it clashes with the field of the same name in the json schema.

Currently looking into how best to resolve this

Copy link

codspeed-hq bot commented Dec 15, 2023

CodSpeed Performance Report

Merging #235 will not alter performance

⚠️ No base runs were found

Falling back to comparing fix-bugs (f4e5d7b) with main (33bcc63)

Summary

✅ 9 untouched benchmarks

Copy link

codecov bot commented Dec 16, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (33bcc63) 86.38% compared to head (f4e5d7b) 86.38%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #235   +/-   ##
=======================================
  Coverage   86.38%   86.38%           
=======================================
  Files          26       26           
  Lines        1256     1256           
=======================================
  Hits         1085     1085           
  Misses        171      171           

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

@tlambert03
Copy link
Owner Author

for now, I'm fixing this problem by expressing choices as a (hashable) tuple of 2-tuples. that does not work for xsdata, but it is easy enough to cast it to a dict in our internal xsdata-pydantic-basemodel handling

@tlambert03 tlambert03 merged commit b070f32 into main Dec 16, 2023
34 checks passed
@tlambert03 tlambert03 deleted the fix-bugs branch December 16, 2023 22:44
@tlambert03 tlambert03 added the bug Something isn't working label Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant