gh-141388: Fully support non-function callables as annotate functions #141449
Conversation
… implement in fwdref format
…ion(format=Format.STRING)`
dr-carlos
left a comment
dr-carlos
left a comment
There was a problem hiding this comment.
Current status: overall, there is a decent amount of special-casing going on to fully support various callable types. But at this point, it supports most stdlib callables (at least, the ones, I could think of), except for C functions which don't have __code__ or __globals__ so can never work with Forwardref/_Stringifier. Some better error messages would be good for those types. More testing is definitely necessary to ensure full support.
Lib/annotationlib.py
Outdated
| if call_func := getattr(annotate.__call__, "__func__", None): | ||
| return getattr(call_func, attr, default) | ||
| elif isinstance(annotate, type): | ||
| return getattr(annotate.__init__, attr, default) |
There was a problem hiding this comment.
This means that only the __init__ method is used for fake global namespaces when a class is used as an annotate function. Maybe the __new__ method should also count? Just adds a bit more complexity.
Lib/annotationlib.py
Outdated
|
|
||
| return default | ||
|
|
||
| def _direct_call_annotate(func, annotate, format): |
There was a problem hiding this comment.
This feels like a poorly named function. Need some way tof describing that we're calling the full annotate function (annotate) based on the part of it that we compiled previously (func) with the given args (usually the format, unless we're recursively calling).
…on function class' __call__ attribute
…et don't raise an error when used as annotate functions
dr-carlos
changed the title
|
I've decided to take this PR out of draft mode as the tests are now pretty comprehensive and every stdlib callable I could think of is now supported where possible.
|
| # Redirect method attribute access to the underlying function. The C code | ||
| # verifies that the __func__ attribute is some kind of callable, so we need | ||
| # to look for attributes recursively. | ||
| if isinstance(annotate, types.MethodType): |
There was a problem hiding this comment.
Sorry, but I don't think we should add all this complexity to support increasingly exotic kinds of callables. Things that quack like a function (in terms of attributes) should work and we should give reasonable errors if something goes wrong, but I don't think we should go out of our way to support various other stdlib callables and try to figure out what the user meant.
There was a problem hiding this comment.
Sure!
No other callables have the exact same attribute layout as functions (hence why this PR exists), but I'm happy to reduce the scope and thus complexity.
Currently this supports:
- Methods of any callable
- Class and generic objects, with any callable as their
__init__method - Callable instances, with any callable as their
__call__method functools.partialandfunctools.partialmethod, wrapping any callable- Wrapped functions/callables
What range of support do you think would be reasonable?
- Normal class methods seem just like functions to the user (albeit bound), support for them would make sense. Plus, this would presumably provide most of the benefit to be gained from supporting non-function annotates.
- Using classes themselves as an annotate function is pretty esoteric, happy to remove this, especially as supporting them introduces the most complexity.
- Most other callables in the stdlib (and I'd bet outside of as well) are simply implemented as Python classes overriding
__call__, so it makes sense to me to support these. functools.partial(but to a lesser extentfunctools.partialmethod) probably 'feel' closest to real functions (to someone unfamiliar with the internals, at least), so I'd be inclined to support these. Happy to only support wrapping normal functions and not arbitrary callables though.- Wrapped functions are already unwrapped in other parts of
annotationliband are pretty easy to support, but if you'd rather not automatically unwrap anything here, I'm happy to remove it.
Anyway, those are my thoughts - please let me know which parts you'd like me to remove/simplify :)
There was a problem hiding this comment.
I don't think any of them should be supported (except to the extent they work because they look like functions).
2 participants
__annotate__function in 3.14, butannotationlibwill start throwing errors if the format is not implemented.call_annotate_function()__code__,__globals__, etc., so aForwardRefor stringification using the current techniques is impossible. Let me know if there are any more callables to implement, some more special-casing will probably be required.annotationlib, but tests are mostly present oncall_annotate_function()for now.__annotate__Functions don't actually need to be functions #141388