Blacklist default globals to support exotic user code with __ prefixed annotations
#10136
Conversation
Deploying pydantic-docs with
|
| Latest commit: |
fca234c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6b878348.pydantic-docs.pages.dev |
| Branch Preview URL: | https://blacklist.pydantic-docs.pages.dev |
CodSpeed Performance ReportMerging #10136 will not alter performanceComparing Summary
|
pydantic/_internal/_typing_extra.py
Outdated
| def _remove_default_globals_from_ns(namespace: dict[str, Any]) -> dict[str, Any]: | ||
| """Remove default globals like __name__, __doc__, etc that aren't needed for type evaluation.""" | ||
| return {k: v for k, v in namespace.items() if not k.startswith(('__', '@'))} | ||
| return {k: v for k, v in namespace.items() if not k.startswith('@') and k not in _DEFAULT_GLOBALS} |
There was a problem hiding this comment.
I'm wondering, @ was taken from pytest-examples but is it really possible to have something starting with that in the namespace? @.* isn't a valid identifier
There was a problem hiding this comment.
Good question, yeah, I saw lots of vars starting with @ in the module namespace when I was running pytest, but perhaps that's not important for real applications. I guess I'm inclined to keep it bc it seems like the performance drawback is negligible, and it ensures the namespace is pretty clean.
|
Actually, I'd like to wait on merging this, I'm seeing this slow down the |
sydney-runkle
changed the title
__ prefixed annotations
|
Alright, removing the double check made it such that we're not seeing any significant performance changes. Going to merge :). |
Coverage report
This PR does not seem to contain any modification to coverable code. |
This way, if someone uses something like
__test__as a type annotation, we don't remove that from the namespace.Contributing to #10074