Skip to content

Conversation

@Viicos
Copy link
Member

@Viicos Viicos commented Sep 6, 2024

Change Summary

I could have implemented this simplification for all cases (perhaps in GenerateSchema._apply_single_annotation or similar) but I don't want to break any unusual scenarios, plus this kind of situation is pretty uncommon in the general case (you're not really going to annotate a field as Any | <other>), it only makes sense when parameterizing T | <other> with Any/Never.

This has the added benefit of creating simplified JSON Schemas, as shown in the test.

Related issue number

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Sep 6, 2024
@Viicos Viicos force-pushed the simplify-any-never-unions branch from f4d6be3 to 9097bd2 Compare September 6, 2024 12:26
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 6, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9097bd2
Status: ✅  Deploy successful!
Preview URL: https://fcd106ae.pydantic-docs.pages.dev
Branch Preview URL: https://simplify-any-never-unions.pydantic-docs.pages.dev

View logs

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 6, 2024

CodSpeed Performance Report

Merging #10338 will not alter performance

Comparing simplify-any-never-unions (9097bd2) with main (c452c7e)

Summary

✅ 49 untouched benchmarks

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2024

Coverage report

Image Image

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic/_internal
  _generics.py ImageImageImageImage
Project Total ImageImageImageImage 

This report was generated by python-coverage-comment-action

Copy link
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks for the inline comments, makes this super easy to review / understand for future devs.

@sydney-runkle sydney-runkle merged commit 287c266 into main Sep 10, 2024
@sydney-runkle sydney-runkle deleted the simplify-any-never-unions branch September 10, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

relnotes-fix Used for bugfixes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants