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

Add support for typing.Self (fix #5992) #9023

Merged
merged 10 commits into from Mar 25, 2024

Conversation

Youssefares
Copy link
Contributor

@Youssefares Youssefares commented Mar 15, 2024

Change Summary

add support for typing.Self introduced in python3.11 as property annotation.

Related issue number

fix #5992

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

Selected Reviewer: @adriangb

@Youssefares Youssefares changed the title allow schema.property: Self Add Mar 15, 2024
@Youssefares Youssefares changed the title Add Add support for typing.Self Mar 15, 2024
@Youssefares Youssefares changed the title Add support for typing.Self Add support for typing.Self (fix #5992) Mar 15, 2024
Copy link

codspeed-hq bot commented Mar 16, 2024

CodSpeed Performance Report

Merging #9023 will not alter performance

Comparing Youssefares:support-typing-self (cd37ddb) with main (534a446)

Summary

✅ 10 untouched benchmarks

@Youssefares
Copy link
Contributor Author

please review

tests/test_types_self.py Outdated Show resolved Hide resolved
Copy link
Member

@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.

Requested a few minor changes. Looks good overall!

@adriangb, are you ok with the stack-based implementation here?

I haven't done a deep dive, but I feel like maybe our 4 (or 5) stacks being passed around might be able to be simplified / unified somewhat...

tests/test_types_self.py Show resolved Hide resolved
tests/test_types_self.py Outdated Show resolved Hide resolved
tests/test_types_self.py Outdated Show resolved Hide resolved
pydantic/_internal/_generate_schema.py Outdated Show resolved Hide resolved
@adriangb
Copy link
Member

I think we maybe need some tests that verify python inheritance behavior, not just data shape. That is, something more along the lines of your original test @Youssefares. Also some json based tests.

@sydney-runkle
Copy link
Member

I'm good with this stack based approach for now. I do think we could revisit consolidating some of the stack based logic, but that need not fit in this PR :).

@sydney-runkle
Copy link
Member

@Youssefares,

If you're able to add those extra test cases that Adrian mentioned by the end of the week, we can include this in the 2.7 release. Thanks a bunch for your help 🚀 this is looking great so far!

@Youssefares
Copy link
Contributor Author

Added more tests. Also realised more work is needed for use of Self inside other types referenced as properties or used with TypeAdapter. So a slight scope expansion with a couple more places to push to the stack and a few more tests.

Feedback appreciated!

Copy link
Member

@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.

@Youssefares,

Great work! Thanks for adding the additional tests for the additional use cases :). This will be a much appreciated feature in 2.7!!

@sydney-runkle sydney-runkle merged commit 426402a into pydantic:main Mar 25, 2024
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Python 3.11 typing.Self type
3 participants