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

Warn if a class inherits from Generic before BaseModel #7891

Merged
merged 2 commits into from Oct 29, 2023

Conversation

alexmojaki
Copy link
Contributor

@alexmojaki alexmojaki commented Oct 21, 2023

Change Summary

Emit a warning when writing:

class Model(Generic[T], BaseModel):

instead of:

class Model(BaseModel, Generic[T]):

More generally, BaseModel must appear in the __mro__ before Generic, so indirect subclasses are also checked. This means that __class_getitem__ will use BaseModel instead of Generic and Model[T] will return a model class rather than a _GenericAlias.

Related issue number

Closes #7845

Waiting for a reply to #6994 (comment) before continuing here, but thought I should push what I have so far. The failing tests might be of interest.

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

@alexmojaki alexmojaki marked this pull request as draft October 21, 2023 12:09
@davidhewitt davidhewitt added the relnotes-change Used for changes to existing functionality which don't have a better categorization. label Oct 24, 2023
@sydney-runkle
Copy link
Member

@alexmojaki,

Thanks for the contribution. I'll discuss this issue with the team tomorrow morning + follow up with you afterwards 😄.

@sydney-runkle
Copy link
Member

@alexmojaki,

Feel free to move ahead with this. We would like to warn if a class inherits from Generic before BaseModel.

@alexmojaki alexmojaki marked this pull request as ready for review October 29, 2023 11:44
@alexmojaki
Copy link
Contributor Author

please review

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.

Nice! Thanks 🌟

@sydney-runkle sydney-runkle merged commit 46f24ce into pydantic:main Oct 29, 2023
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review relnotes-change Used for changes to existing functionality which don't have a better categorization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn about inheriting from Generic before BaseModel
4 participants