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.

Sign up for GitHub

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

Jump to bottom

Discriminated union schemas use oneOf instead of anyOf #4335

Merged
merged 1 commit into from Aug 11, 2022
Merged

Discriminated union schemas use oneOf instead of anyOf #4335

merged 1 commit into from Aug 11, 2022

Conversation

MaxwellPayne
Copy link
Contributor

@MaxwellPayne MaxwellPayne commented Aug 6, 2022 •
edited

Change Summary

Discriminated union models now use oneOf instead of anyOf when generating OpenAPI schema definitions.

Related issue number

Fixes #4125

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@MaxwellPayne MaxwellPayne marked this pull request as ready for review August 6, 2022 05:00
@MaxwellPayne
Copy link
Contributor Author

I opened this PR as a follow-up to #4125. I saw @stdkoehler's comments on that issue and agreed that oneOf is a more accurate description of discriminated union within OpenAPI than is anyOf. My solution will still output anyOf in situations where no discriminator is involved, but should always output oneOf when the union values can be determined by a discriminator field. Please review.

This was referenced Aug 6, 2022
Closed
Closed
@samuelcolvin
Copy link
Member

JSONSchema's oneOf/anyOf distinction is daft, I really can't imagine a scenario where it's helpful. Ignoring the extra issue, I can't imagine why you would ever care if data was valid for only one choice???

It seems changes is mostly motivated by a need for compatibility with other tools that use oneOf, not a desire for semantic correctness.

Still I can see the argument in the case of discriminated unions. I'm happy to accept this, please update to fix conflicts.

@MaxwellPayne
Copy link
Contributor Author

JSONSchema's oneOf/anyOf distinction is daft, I really can't imagine a scenario where it's helpful. Ignoring the extra issue, I can't imagine why you would ever care if data was valid for only one choice???

Couldn't agree more; it took me a while to figure out what was going on here.

Still I can see the argument in the case of discriminated unions. I'm happy to accept this, please update to fix conflicts.

Thanks for your consideration! I just resolved the conflicts, please review at your convenience.

@samuelcolvin samuelcolvin merged commit 4cb58cf into pydantic:master Aug 11, 2022
@samuelcolvin
Copy link
Member

thank you so much.

@MaxwellPayne MaxwellPayne deleted the discriminated-union-oneof branch August 11, 2022 16:56
@HIL340 HIL340 mentioned this pull request Dec 7, 2022
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@hramezani hramezani

@samuelcolvin samuelcolvin

@PrettyWood PrettyWood

Labels
ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Union produces anyOf instead of oneOf for OpenAPI generation
4 participants
@MaxwellPayne @samuelcolvin @hramezani @PrettyWood