Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
While this change is correct to include
UnionType, the subsequent logic for handlingOptional[T]types (which results inlen(schema.any_of) == 1) is a bit fragile and could be improved for robustness.The current implementation at lines 369-371 only copies the
typefrom the wrapped schema, and relies on a special-case handling forlisttypes at lines 352-362 to copy theitems. This means that for other complexOptionaltypes, likeOptional[pydantic.BaseModel], properties will be lost during the unwrapping.Consider refactoring the unwrapping logic to be more generic and robust. For example:
This would handle various
Optionaltypes correctly and would also allow removing the special-case logic for lists.