Skip to content

Warn if a class inherits from Generic before BaseModel#7891

Merged
sydney-runkle merged 2 commits intopydantic:mainfrom
alexmojaki:GenericBeforeBaseModelWarning
Oct 29, 2023
Merged

Warn if a class inherits from Generic before BaseModel#7891
sydney-runkle merged 2 commits intopydantic:mainfrom
alexmojaki:GenericBeforeBaseModelWarning

Conversation

@alexmojaki
Copy link
Copy Markdown
Contributor

@alexmojaki alexmojaki commented Oct 21, 2023 •
edited by pydantic-hooky Bot
Loading

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
Copy Markdown
Contributor

sydney-runkle commented Oct 25, 2023

@alexmojaki,

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

@sydney-runkle
Copy link
Copy Markdown
Contributor

sydney-runkle commented Oct 25, 2023

@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
Copy Markdown
Contributor Author

alexmojaki commented Oct 29, 2023

please review

sydney-runkle
sydney-runkle approved these changes Oct 29, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Nice! Thanks 🌟

@sydney-runkle sydney-runkle merged commit 46f24ce into pydantic:main Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@sydney-runkle sydney-runkle sydney-runkle approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dmontagu dmontagu

Labels

ready for review relnotes-change Used for changes to existing functionality which don't have a better categorization.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Warn about inheriting from Generic before BaseModel

4 participants

@alexmojaki @sydney-runkle @davidhewitt @dmontagu