Skip to content

annotationlib: namespace can be non-dict #132426

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

Open
eliegoudout opened this issue Apr 12, 2025 · 3 comments · May be fixed by #132427
Open

annotationlib: namespace can be non-dict #132426

eliegoudout opened this issue Apr 12, 2025 · 3 comments · May be fixed by #132427
Labels
3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error

Comments

@eliegoudout
Copy link

eliegoudout commented Apr 12, 2025

Hello!

I think the following, made so that get_annotate_function is usable in metaclasses' __new__, should check for Mapping since __prepare__ can provide any mapping (see picture from PEP3115.

if isinstance(obj, dict):

Image

Linked PRs

eliegoudout added a commit to eliegoudout/cpython that referenced this issue Apr 12, 2025
Signed-off-by: Élie Goudout <eliegoudout@hotmail.com>
@eliegoudout
Copy link
Author

eliegoudout commented Apr 12, 2025

However, I now realize that this is a major issue since there might be a conflict between the Mapping subtype's own annotations and the annotations it contains as a mapping for another type's namespace.

Shouldn't we separate get_annotate_function into get_annotate_function and get_namespace_annotations? Or maybe add a keyword get_annotate_function(obj, *, is_namespace: bool)?

@picnixz picnixz added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir topic-typing labels Apr 12, 2025
@picnixz
Copy link
Member

picnixz commented Apr 12, 2025

cc @JelleZijlstra

@picnixz picnixz added the 3.14 new features, bugs and security fixes label Apr 12, 2025
@JelleZijlstra JelleZijlstra pinned this issue Apr 12, 2025
@JelleZijlstra JelleZijlstra unpinned this issue Apr 12, 2025
@JelleZijlstra
Copy link
Member

Good point! I think what I'd like to do is drop get_annotate_function completely (it's now equivalent to a simple getattr call and doesn't need to be a function) and add a new function get_annotate_from_class_namespace.

Your proposed change is a bit dubious because "mapping" at the C level means something a little different (it basically just checks for the mp_subscr slot). Not sure if that will cause practical problems but better to be safe.

However, I now realize that this is a major issue since there might be a conflict between the Mapping subtype's own annotations and the annotations it contains as a mapping for another type's namespace.

Since instances no longer have accessible annotations by default, this will only be a problem in exotic cases like a Mapping that is also an instance of type. But again, better to be safe.

JelleZijlstra added a commit to JelleZijlstra/cpython that referenced this issue Apr 14, 2025
…annotate_function

As noted on the issue, making get_annotate_function() support both types and
mappings is problematic because one object may be both. So let's add a new one
that works with any mapping.

This leaves get_annotate_function() not very useful, so remove it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants