Skip to content

Conversation

Viicos
Copy link
Member

@Viicos Viicos commented Dec 15, 2024

We avoid relying on ABCMeta.__subclasscheck__ unless we're pretty sure. Note that this was wrongly addressed as __instancecheck__ was implemented instead of __subclasscheck__. However, it is not clear if implementing __instancecheck__ fixed existing issues as well, so it is left as is.

Also changed the checked attribute to __pydantic_decorators__, as this is one of the first attributes being set on the class, and it might be that we change the way __pydantic_validator__ is set in the future (e.g. when defer_build is set, and if we don't implement mock validators anymore).

Fixes #11100.

Change Summary

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

… and performance issues

We avoid relying on `ABCMeta.__subclasscheck__` unless we're pretty sure.
Note that this was wrongly addressed as `__instancecheck__` was implemented
instead of `__subclasscheck__`. However, it is not clear if implementing
`__instancecheck__` fixed existing issues as well, so it is left as is.

Also changed the checked attribute to `__pydantic_decorators__`, as
this is one of the first attributes being set on the class, and it
might be that we change the way `__pydantic_validator__` is set in
the future (e.g. when `defer_build` is set, and if we don't implement
mock validators anymore).
@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Dec 15, 2024
Copy link

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1922dfe
Status: ✅  Deploy successful!
Preview URL: https://41a7f040.pydantic-docs.pages.dev
Branch Preview URL: https://subclasscheck.pydantic-docs.pages.dev

View logs

Copy link

codspeed-hq bot commented Dec 15, 2024

CodSpeed Performance Report

Merging #11116 will not alter performance

Comparing subclasscheck (1922dfe) with main (d5f4bde)

Summary

✅ 46 untouched benchmarks

Copy link
Contributor

Coverage report

Click to see where and how coverage changed

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

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.

Clean. Nice. Thanks! Could you add a test before we merge?

@Viicos
Copy link
Member Author

Viicos commented Dec 16, 2024

What kind of test are you looking for? We already have test_model_issubclass

@sydney-runkle sydney-runkle merged commit 95c374d into main Dec 16, 2024
55 checks passed
@sydney-runkle sydney-runkle deleted the subclasscheck branch December 16, 2024 23:15
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.

Memory leak with issubclass(x, BaseModel)
2 participants