Skip to content

Conversation

max-muoto
Copy link
Contributor

@max-muoto max-muoto commented Apr 28, 2024

Change Summary

Currently the typing for create_model leads to unknowns, possibly raising errors for Pyright users. This is because classmethod is a generic class, and we're simply not passing in any generic parameters for it.

This PR fixes the Pyright errors here by passing in generics for usages of classmethod here. My presumption is that the validator does not have to return the same type as it accepts, but if that assumption is wrong, I can switch this to use generics for first argument/return value. I use ... for the second value since it's simply a ParamSpec, and we don't really have a way to signify it takes in a singular argument unless we want to switch this to use Callable.

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: @samuelcolvin

Copy link

codspeed-hq bot commented Apr 28, 2024

CodSpeed Performance Report

Merging #9338 will not alter performance

Comparing max-muoto:improve-create-model-typing (688ae29) with main (c4d2edc)

Summary

✅ 13 untouched benchmarks

@max-muoto max-muoto force-pushed the improve-create-model-typing branch from 693b1cb to 2f0579e Compare April 28, 2024 18:47
@max-muoto max-muoto marked this pull request as ready for review April 28, 2024 18:48
@max-muoto max-muoto changed the title Improve typing for create_model Provide missing generics for classmethod in create_model Apr 28, 2024
@max-muoto max-muoto force-pushed the improve-create-model-typing branch from 2f0579e to 688ae29 Compare April 28, 2024 18:59
@max-muoto
Copy link
Contributor Author

please review

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.

Great, thanks! Your assumption re validators is correct :).

@sydney-runkle sydney-runkle added the relnotes-ignore Omit this PR from the release notes. label Apr 28, 2024
@sydney-runkle sydney-runkle merged commit 37fa1bc into pydantic:main Apr 28, 2024
@max-muoto
Copy link
Contributor Author

Great, thanks! Your assumption re validators is correct :).

Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review relnotes-ignore Omit this PR from the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants