gh-85403: Make wraps retain type annotations #21392
Conversation
When decorating a function with ``wraps``, retain the type annotations of the wrapper if there's any, and use the wrapped function ones to complete the annotations by default. This changes the ``update_wrapper`` function to use the wrapper function annotations if any, and fall back to the wrapped function annotations if the wrapper one did not have them. For the return type annotations, and the parameter types annotations independently. The annotations overriding behavior was introduced in bpo-8814.
Signed-off-by: David Caro <david@dcaro.es>
terryjreedy
changed the title
|
Conflict was trivial. Do a simple update merge of main into the branch rather than a rebase. |
|
Hmm, how does this interact with pep 612? For toy example, from functools import wraps
def log_info(func: Callable[P, R]) -> Callable[P, R]:
@wraps
def _log_func(*args: P.args, **kwargs: P.kwargs) -> R:
print("stuff")
return func(*args, **kwargs)
return _log_func
@log_info
def foo(x: int) -> str:
...The current behavior of wrap is very different here than new behavior. Paramspecs are intended to preserve original function signature and not overrwrite it. I know that code that uses decorators with beam library's runtime time introspection would be affected by this change in a way that can cause it to fail to work. beam inspects function signature types and doesn't handle paramspecs. If both original function (foo) and inner wrapper function (_log_func) have all type hints available still at runtime then I think this is possible to solve, but a very easy edge case bug for any runtime type library. |
I was quite confused by the docs (which make things much more complicated than the code).
| ``assigned`` list parameter), it will be populated only if missing some | ||
| annotations as follows: | ||
|
|
||
| * If the wrapper function defines it's own return type and parameter |
There was a problem hiding this comment.
| * If the wrapper function defines it's own return type and parameter | |
| * If the wrapper function defines its own return type and parameter |
| annotations, those will be preserved. | ||
|
|
||
| * If the wrapper function does not annotate the return type, it will be | ||
| copied from the wrapped if any. |
There was a problem hiding this comment.
| copied from the wrapped if any. | |
| copied from the wrapped function, if it specifies a return type. |
| When assigning the ``__annotations__`` attribute (when it's part of the | ||
| ``assigned`` list parameter), it will be populated only if missing some |
There was a problem hiding this comment.
Is it not part of WRAPPER_ASSIGNMENTS?
| copied from the wrapped if any. | ||
|
|
||
| * If the wrapped function does not annotate any parameters, the annotations | ||
| for them will be copied over from the wrapped if any. |
There was a problem hiding this comment.
| for them will be copied over from the wrapped if any. | |
| for them will be copied over from the wrapped function, if it has any. |
| * If the wrapper function defines it's own return type and parameter | ||
| annotations, those will be preserved. |
There was a problem hiding this comment.
It is not very clear from this paragraph what this actually does.
Suppose the wrapper has annotations for 'x' and 'y' and the wrapped function has annotations for 'z'. Does 'z' get added to the wrapper's annotations?
Suppose the wrapper has no annotations, so it receives annotations for 'x', 'y' and 'z' from the wrapped function. What happens if the wrapper doesn't have arguments by those names? That is going to be confusing in some cases of further introspection.
It almost sounds like __annotations__ should be listed under updated instead of assigned, there are so many special cases.
| That ensures that any custom annotations from the wrapper will not be | ||
| overridden by the wrapped, allowing to change the annotated types of | ||
| the wrapped function. | ||
|
|
||
| .. versionadded:: 3.2 |
There was a problem hiding this comment.
Clearly you need a versionadded:: 3.12 block for the new treatment of __annotations__.
|
|
||
| if '__annotations__' in assigned: | ||
| try: | ||
| # Issue #41231: copy the annotations from wrapped, only if they are |
There was a problem hiding this comment.
Update the bug number using the GH-NNN or bpo-NNN format.
| # if the wrapper does not annotate the return type, copy the | ||
| # annotation from the wrapped | ||
| elif ('return' not in wrapper.__annotations__ | ||
| and 'return' in wrapped.__annotations__): | ||
| wrapper.__annotations__['return'] = wrapped.__annotations__['return'] |
There was a problem hiding this comment.
Oh this is quite different from what I had understood from the docs (which I read before reading the code). I recommend changing the docs to more clearly describe what this does.
That's a really good point. I'll leave it to the patch author to respond, but without a good answer (and backwards compatible behavior, given that ParamSpec is in 3.10 and 3.11) this PR can't go through. |
When decorating a function with
wraps, retain the type annotationsof the wrapper if there's any, and use the wrapped function ones to
complete the annotations by default.
This changes the
update_wrapperfunction to use the wrapper functionannotations if any, and fall back to the wrapped function annotations if the
wrapper one did not have them. For the return type annotations, and the
parameter types annotations independently.
The annotations overriding behavior was introduced in bpo-8814.
https://bugs.python.org/issue41231