Fix schema generation for nested/repeated models #333
Conversation
Remove definitions update
| m_schema, _ = model_process_schema( | ||
| model, model_name_map=model_name_map, ref_prefix=REF_PREFIX | ||
| ) | ||
| definitions.update(m_definitions) |
There was a problem hiding this comment.
This update is not necessary given the function is already iterating over flat_models. Moreover, under the right circumstances, it will overwrite valid definitions with None. I've added a test for this below.
| definitions: Dict[str, Dict] = {} | ||
| for model in flat_models: | ||
| m_schema, m_definitions = model_process_schema( | ||
| for model in sorted(flat_models, key=lambda x: x.__name__): |
There was a problem hiding this comment.
I added the sorting (by name) here to ensure reproducibility of the test I added. I assume the cost of this sort is negligible for practical purposes (given this is just used to generate the schema).
Codecov Report
@@ Coverage Diff @@
## master #333 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 232 233 +1
Lines 5492 5505 +13
=====================================
+ Hits 5492 5505 +13
Continue to review full report at Codecov.
|
| def f(): | ||
| pass | ||
|
|
||
| openapi = app.openapi() |
There was a problem hiding this comment.
The test will fail at this line if the definitions.update(m_definitions) is not removed above.
|
@tiangolo it looks like this is addressed by pydantic/pydantic#621 -- if that's right I'll close this pull request, but I think it could still make sense to add the schema-generation test I wrote here. (I don't care about the attribution here so feel free to just copy the test into another PR if you want it.) Note that the schema generation is currently not deterministic due to iteration over the set of models; it might make sense to change this for the sake of testability (given the minimal performance implications). |
|
Thanks @dmontagu ! I'll come back to this once we have a new Pydantic release with pydantic/pydantic#621. We can leave this open, that will remind me to include your test. |
Remove definitions update
Addresses #332