From 20beb4aec8d8257246d78588bf100c7934a425ef Mon Sep 17 00:00:00 2001 From: Lucas Wiman Date: Fri, 24 Jun 2022 23:16:42 -0700 Subject: [PATCH 01/12] Add failing test reproducing #85. --- tests/issue_85_module.py | 10 ++++++++++ tests/test_issues.py | 16 ++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 tests/issue_85_module.py diff --git a/tests/issue_85_module.py b/tests/issue_85_module.py new file mode 100644 index 0000000..34ef57c --- /dev/null +++ b/tests/issue_85_module.py @@ -0,0 +1,10 @@ +from dataclasses import dataclass + + +def forwardref_method(foo: "ForwardRef", bar: str) -> "ForwardRef": + return foo.x + bar + + +@dataclass +class ForwardRef: + x: str = "default" diff --git a/tests/test_issues.py b/tests/test_issues.py index 912f9df..6ce69b4 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -253,3 +253,19 @@ def test_issue_77_async_generator_partial(): assert inspect.isasyncgenfunction(f_partial) assert asyncio.get_event_loop().run_until_complete(asyncio.ensure_future(f_partial().__anext__())) == 1 + + +def test_issue_85_wrapped_forwardref_annotation(): + import typing + from . import issue_85_module + + @wraps(issue_85_module.forwardref_method, remove_args=["bar"]) + def decorated(*args, **kwargs): + return issue_85_module.forwardref_method(*args, **kwargs, bar="x") + + assert decorated(issue_85_module.ForwardRef()) == "defaultx" + expected_annotations = { + "foo": issue_85_module.ForwardRef, + "return": issue_85_module.ForwardRef, + } + assert typing.get_type_hints(decorated) == expected_annotations From f93d2b4817a0b3c5ecccf290fe8f7032f4162d5d Mon Sep 17 00:00:00 2001 From: Lucas Wiman Date: Fri, 24 Jun 2022 23:55:42 -0700 Subject: [PATCH 02/12] Fix test for #85, and update test for #66. I am not sure the fix for #66 was correct given that __wrapped__ is used by the typing module in get_type_hints, and setting __signature__ seems to fix all other tests. --- src/makefun/main.py | 20 +++++++++----------- tests/test_issues.py | 5 ++--- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/makefun/main.py b/src/makefun/main.py index 14c7b0e..f8448e6 100644 --- a/src/makefun/main.py +++ b/src/makefun/main.py @@ -246,6 +246,8 @@ def create_function(func_signature, # type: Union[str, Signature] if isinstance(func_signature, str): # transform the string into a Signature and make sure the string contains ":" func_name_from_str, func_signature, func_signature_str = get_signature_from_string(func_signature, evaldict) + if '__signature__' in attrs: + attrs['__signature__'] = func_signature # if not explicitly overridden using `func_name`, the name in the string takes over if func_name_from_str is not None: @@ -819,9 +821,11 @@ def wraps(wrapped_fun, `wrapped_fun`, so that the created function seems to be identical (except possiblyfor the signature). Note that all options in `with_signature` can still be overrided using parameters of `@wraps`. - If the signature is *not* modified through `new_sig`, `remove_args`, `append_args` or `prepend_args`, the - additional `__wrapped__` attribute on the created function, to stay consistent with the `functools.wraps` - behaviour. + The additional `__wrapped__` attribute is set on the created function, to stay consistent + with the `functools.wraps` behaviour. If the signature is modified through `new_sig`, + `remove_args`, `append_args` or `prepend_args`, the additional + `__signature__` attribute will be set so that `inspect.signature` and related functionality + works as expected. See PEP 362 for more detail on `__wrapped__` and `__signature__`. See also [python documentation on @wraps](https://docs.python.org/3/library/functools.html#functools.wraps) @@ -960,15 +964,9 @@ def _get_args_for_wrapping(wrapped, new_sig, remove_args, prepend_args, append_a # attributes: start from the wrapped dict, add '__wrapped__' if needed, and override with all attrs. all_attrs = copy(getattr_partial_aware(wrapped, '__dict__')) + all_attrs.setdefault("__wrapped__", wrapped) if has_new_sig: - # change of signature: delete the __wrapped__ attribute if any - try: - del all_attrs['__wrapped__'] - except KeyError: - pass - else: - # no change of signature: we can safely set the __wrapped__ attribute - all_attrs['__wrapped__'] = wrapped + all_attrs["__signature__"] = func_sig all_attrs.update(attrs) return func_name, func_sig, doc, qualname, co_name, module_name, all_attrs diff --git a/tests/test_issues.py b/tests/test_issues.py index 6ce69b4..d0f7c56 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -181,11 +181,10 @@ def wrapper(foo): def second_wrapper(foo, bar): return wrapper(foo) + bar + assert second_wrapper.__wrapped__ is a + assert "bar" in inspect.signature(second_wrapper).parameters assert second_wrapper(1, -1) == 0 - with pytest.raises(AttributeError): - second_wrapper.__wrapped__ - def test_issue_pr_67(): """Test handcrafted for https://github.com/smarie/python-makefun/pull/67""" From 9041de2d5466f2ead8cfaf7b44e48655fee45b1f Mon Sep 17 00:00:00 2001 From: "[object Object]" Date: Sat, 25 Jun 2022 10:57:23 -0700 Subject: [PATCH 03/12] Update src/makefun/main.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sylvain Marié --- src/makefun/main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/makefun/main.py b/src/makefun/main.py index f8448e6..c45d017 100644 --- a/src/makefun/main.py +++ b/src/makefun/main.py @@ -964,6 +964,7 @@ def _get_args_for_wrapping(wrapped, new_sig, remove_args, prepend_args, append_a # attributes: start from the wrapped dict, add '__wrapped__' if needed, and override with all attrs. all_attrs = copy(getattr_partial_aware(wrapped, '__dict__')) + # PEP362: always set `__wrapped__`, and if signature was changed, set `__signature__` too all_attrs.setdefault("__wrapped__", wrapped) if has_new_sig: all_attrs["__signature__"] = func_sig From 7ef419126a5b8c5b575ff60ee4355030cc9adfee Mon Sep 17 00:00:00 2001 From: "[object Object]" Date: Sat, 25 Jun 2022 10:57:41 -0700 Subject: [PATCH 04/12] Update tests/test_issues.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sylvain Marié --- tests/test_issues.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_issues.py b/tests/test_issues.py index d0f7c56..beaafd4 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -263,6 +263,8 @@ def decorated(*args, **kwargs): return issue_85_module.forwardref_method(*args, **kwargs, bar="x") assert decorated(issue_85_module.ForwardRef()) == "defaultx" + + # Check that the type hints of the wrapper are ok with the forward reference correctly resolved expected_annotations = { "foo": issue_85_module.ForwardRef, "return": issue_85_module.ForwardRef, From 95f7697d3416a15f85fb789826c5604ec8799cfa Mon Sep 17 00:00:00 2001 From: "[object Object]" Date: Sat, 25 Jun 2022 10:58:03 -0700 Subject: [PATCH 05/12] Update tests/test_issues.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sylvain Marié --- tests/test_issues.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_issues.py b/tests/test_issues.py index beaafd4..f5c3ee6 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -262,6 +262,7 @@ def test_issue_85_wrapped_forwardref_annotation(): def decorated(*args, **kwargs): return issue_85_module.forwardref_method(*args, **kwargs, bar="x") + # Make sure the wrapper function works as expected assert decorated(issue_85_module.ForwardRef()) == "defaultx" # Check that the type hints of the wrapper are ok with the forward reference correctly resolved From 5fc7b1c06e2f0f6fd5c37f857e858388494fb9d8 Mon Sep 17 00:00:00 2001 From: "[object Object]" Date: Sat, 25 Jun 2022 11:09:16 -0700 Subject: [PATCH 06/12] Update src/makefun/main.py --- src/makefun/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/makefun/main.py b/src/makefun/main.py index c45d017..02508a3 100644 --- a/src/makefun/main.py +++ b/src/makefun/main.py @@ -965,7 +965,7 @@ def _get_args_for_wrapping(wrapped, new_sig, remove_args, prepend_args, append_a # attributes: start from the wrapped dict, add '__wrapped__' if needed, and override with all attrs. all_attrs = copy(getattr_partial_aware(wrapped, '__dict__')) # PEP362: always set `__wrapped__`, and if signature was changed, set `__signature__` too - all_attrs.setdefault("__wrapped__", wrapped) + all_attrs["__wrapped__"] = wrapped if has_new_sig: all_attrs["__signature__"] = func_sig all_attrs.update(attrs) From d9e9e3f8639bdfa7d0b7629d0cff2528a10ea489 Mon Sep 17 00:00:00 2001 From: Lucas Wiman Date: Sat, 25 Jun 2022 11:23:56 -0700 Subject: [PATCH 07/12] Fix tests. 1. Always setting __wrapped__ means we needed a slightly different assertion. 2. Support python <3.7 in the test suite. --- tests/issue_85_module.py | 10 ++++------ tests/test_issues.py | 13 ++++++++----- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/tests/issue_85_module.py b/tests/issue_85_module.py index 34ef57c..79b521b 100644 --- a/tests/issue_85_module.py +++ b/tests/issue_85_module.py @@ -1,10 +1,8 @@ -from dataclasses import dataclass - - def forwardref_method(foo: "ForwardRef", bar: str) -> "ForwardRef": - return foo.x + bar + return ForwardRef(foo.x + bar) -@dataclass class ForwardRef: - x: str = "default" + x: str + def __init__(self, x="default"): + self.x = x diff --git a/tests/test_issues.py b/tests/test_issues.py index f5c3ee6..e041649 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -181,7 +181,7 @@ def wrapper(foo): def second_wrapper(foo, bar): return wrapper(foo) + bar - assert second_wrapper.__wrapped__ is a + assert second_wrapper.__wrapped__ is wrapper assert "bar" in inspect.signature(second_wrapper).parameters assert second_wrapper(1, -1) == 0 @@ -254,20 +254,23 @@ def test_issue_77_async_generator_partial(): assert asyncio.get_event_loop().run_until_complete(asyncio.ensure_future(f_partial().__anext__())) == 1 + +@pytest.mark.skipif(sys.version_info < (3, 3), reason="Tests PEP 367 behavior, which was introduced in python 3.3.") def test_issue_85_wrapped_forwardref_annotation(): import typing from . import issue_85_module @wraps(issue_85_module.forwardref_method, remove_args=["bar"]) - def decorated(*args, **kwargs): - return issue_85_module.forwardref_method(*args, **kwargs, bar="x") + def wrapper(**kwargs): + kwargs["bar"] = "x" # python 2 syntax to prevent syntax error. + return issue_85_module.forwardref_method(**kwargs) # Make sure the wrapper function works as expected - assert decorated(issue_85_module.ForwardRef()) == "defaultx" + assert wrapper(issue_85_module.ForwardRef()).x == "defaultx" # Check that the type hints of the wrapper are ok with the forward reference correctly resolved expected_annotations = { "foo": issue_85_module.ForwardRef, "return": issue_85_module.ForwardRef, } - assert typing.get_type_hints(decorated) == expected_annotations + assert typing.get_type_hints(wrapper) == expected_annotations From 9c54231401ae7aca330c4da23a4734a7b67053b9 Mon Sep 17 00:00:00 2001 From: Lucas Wiman Date: Sat, 25 Jun 2022 12:00:45 -0700 Subject: [PATCH 08/12] Update version specifier to be more specific. --- tests/issue_85_module.py | 1 - tests/test_issues.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/issue_85_module.py b/tests/issue_85_module.py index 79b521b..7ab3790 100644 --- a/tests/issue_85_module.py +++ b/tests/issue_85_module.py @@ -3,6 +3,5 @@ def forwardref_method(foo: "ForwardRef", bar: str) -> "ForwardRef": class ForwardRef: - x: str def __init__(self, x="default"): self.x = x diff --git a/tests/test_issues.py b/tests/test_issues.py index e041649..370df76 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -255,7 +255,7 @@ def test_issue_77_async_generator_partial(): -@pytest.mark.skipif(sys.version_info < (3, 3), reason="Tests PEP 367 behavior, which was introduced in python 3.3.") +@pytest.mark.skipif(sys.version_info < (3, 7, 6), reason="The __wrapped__ behavior in get_type_hints being tested was not added until python 3.7.6.") def test_issue_85_wrapped_forwardref_annotation(): import typing from . import issue_85_module From 4f5c5ca692cc856229240ac9d8afc2cfac90d0dd Mon Sep 17 00:00:00 2001 From: Lucas Wiman Date: Sat, 25 Jun 2022 12:13:54 -0700 Subject: [PATCH 09/12] Get test suite passing on 2.7. 1. Rename issue_85_module so that it is not collected by pytest. 2. Use the signature polyfill rather than inspect.signature. --- tests/{issue_85_module.py => _issue_85_module.py} | 0 tests/test_issues.py | 14 +++++++------- 2 files changed, 7 insertions(+), 7 deletions(-) rename tests/{issue_85_module.py => _issue_85_module.py} (100%) diff --git a/tests/issue_85_module.py b/tests/_issue_85_module.py similarity index 100% rename from tests/issue_85_module.py rename to tests/_issue_85_module.py diff --git a/tests/test_issues.py b/tests/test_issues.py index 370df76..87a6c1a 100644 --- a/tests/test_issues.py +++ b/tests/test_issues.py @@ -182,7 +182,7 @@ def second_wrapper(foo, bar): return wrapper(foo) + bar assert second_wrapper.__wrapped__ is wrapper - assert "bar" in inspect.signature(second_wrapper).parameters + assert "bar" in signature(second_wrapper).parameters assert second_wrapper(1, -1) == 0 @@ -258,19 +258,19 @@ def test_issue_77_async_generator_partial(): @pytest.mark.skipif(sys.version_info < (3, 7, 6), reason="The __wrapped__ behavior in get_type_hints being tested was not added until python 3.7.6.") def test_issue_85_wrapped_forwardref_annotation(): import typing - from . import issue_85_module + from . import _issue_85_module - @wraps(issue_85_module.forwardref_method, remove_args=["bar"]) + @wraps(_issue_85_module.forwardref_method, remove_args=["bar"]) def wrapper(**kwargs): kwargs["bar"] = "x" # python 2 syntax to prevent syntax error. - return issue_85_module.forwardref_method(**kwargs) + return _issue_85_module.forwardref_method(**kwargs) # Make sure the wrapper function works as expected - assert wrapper(issue_85_module.ForwardRef()).x == "defaultx" + assert wrapper(_issue_85_module.ForwardRef()).x == "defaultx" # Check that the type hints of the wrapper are ok with the forward reference correctly resolved expected_annotations = { - "foo": issue_85_module.ForwardRef, - "return": issue_85_module.ForwardRef, + "foo": _issue_85_module.ForwardRef, + "return": _issue_85_module.ForwardRef, } assert typing.get_type_hints(wrapper) == expected_annotations From a485b91fae7b07bbe614b8e28ea2613981fa2fc7 Mon Sep 17 00:00:00 2001 From: "[object Object]" Date: Tue, 5 Jul 2022 11:20:24 -0700 Subject: [PATCH 10/12] Update src/makefun/main.py --- src/makefun/main.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/makefun/main.py b/src/makefun/main.py index 02508a3..6735861 100644 --- a/src/makefun/main.py +++ b/src/makefun/main.py @@ -246,6 +246,9 @@ def create_function(func_signature, # type: Union[str, Signature] if isinstance(func_signature, str): # transform the string into a Signature and make sure the string contains ":" func_name_from_str, func_signature, func_signature_str = get_signature_from_string(func_signature, evaldict) + # If the signature is specified as a string, `__signature__` will not be correct in this case, since it + # will be a string. If it was computed in some other fashion (i.e. is an instance of inspect.Signature), + # then there is no need to update it. if '__signature__' in attrs: attrs['__signature__'] = func_signature From ccc922a5ab60922e0728488e4e59a9088b47fed4 Mon Sep 17 00:00:00 2001 From: Lucas Wiman Date: Tue, 5 Jul 2022 11:29:56 -0700 Subject: [PATCH 11/12] Update documentation to match new PEP 362 behavior. --- docs/api_reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api_reference.md b/docs/api_reference.md index 2300796..fbc6040 100644 --- a/docs/api_reference.md +++ b/docs/api_reference.md @@ -166,7 +166,7 @@ Comparison with `@with_signature`: `@wraps(f)` is equivalent to In other words, as opposed to `@with_signature`, the metadata (doc, module name, etc.) is provided by the wrapped `wrapped_fun`, so that the created function seems to be identical (except possiblyfor the signature). Note that all options in `with_signature` can still be overrided using parameters of `@wraps`. -If the signature is *not* modified through `new_sig`, `remove_args`, `append_args` or `prepend_args`, the additional `__wrapped__` attribute on the created function, to stay consistent with the `functools.wraps` behaviour. +The additional `__wrapped__` attribute is added on the created function, to stay consistent with the `functools.wraps` behaviour. If the signature is modified through `new_sig`, `remove_args`, `append_args` or `prepend_args`, the `__signature__` attribute will be added per [PEP 362](https://peps.python.org/pep-0362/). See also [python documentation on @wraps](https://docs.python.org/3/library/functools.html#functools.wraps) From 0fe6a7e067c74b791b545f36bc6224f528c92344 Mon Sep 17 00:00:00 2001 From: Lucas Wiman Date: Tue, 6 Sep 2022 16:41:55 -0700 Subject: [PATCH 12/12] Always evaluate the signature when __signature__ is specified as a string. --- src/makefun/main.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/makefun/main.py b/src/makefun/main.py index 6735861..6228ec7 100644 --- a/src/makefun/main.py +++ b/src/makefun/main.py @@ -246,11 +246,6 @@ def create_function(func_signature, # type: Union[str, Signature] if isinstance(func_signature, str): # transform the string into a Signature and make sure the string contains ":" func_name_from_str, func_signature, func_signature_str = get_signature_from_string(func_signature, evaldict) - # If the signature is specified as a string, `__signature__` will not be correct in this case, since it - # will be a string. If it was computed in some other fashion (i.e. is an instance of inspect.Signature), - # then there is no need to update it. - if '__signature__' in attrs: - attrs['__signature__'] = func_signature # if not explicitly overridden using `func_name`, the name in the string takes over if func_name_from_str is not None: @@ -283,6 +278,11 @@ def create_function(func_signature, # type: Union[str, Signature] else: raise TypeError("Invalid type for `func_signature`: %s" % type(func_signature)) + if isinstance(attrs.get('__signature__'), str): + # __signature__ must be a Signature object, so if it is a string, + # we need to evaluate it. + attrs['__signature__'] = get_signature_from_string(attrs['__signature__'], evaldict)[1] + # extract all information needed from the `Signature` params_to_kw_assignment_mode = get_signature_params(func_signature) params_names = list(params_to_kw_assignment_mode.keys())