python-makefun
python-makefun copied to clipboard
Use `__signature__` and set `__wrapped__` attribute even on signature-changing decorators.
Fixes #85 by using the __signature__ attribute specified by PEP 362 where appropriate, and setting the __wrapped__ attribute in all cases.
I think the fix for #66 introduced an issue with using forwardrefs in type annotations. Since typing uses __wrapped__ to dereference forward references, it's can lead to inconsistent behavior (https://github.com/python/cpython/blob/605e9c66ad367b54a847f9fc65447a071742f554/Lib/typing.py#L2314-L2315), not setting it seems inappropriate.
Always setting __wrapped__ and setting __signature__ when the signature changes seems to fix all the issues with typing and docs/etc.
Testing
I added a failing test for #85, asserting that typing.get_type_hints works correctly with signature-changing decorator on a method with a forward reference in its type annotations. It passes after my fix.
Very nice proposal @lucaswiman !
It seems that your test code is not compliant with python 2, 3.5, and 3.6, mostly because you use dataclasses and type hints in the dataclass. I guess this is optional, since forward refs and type hints exist since 3.5
If you believe that this is not worth trying to pass on python 3.6 or 3.5, please move your test module contents in a new file named _test_py37. Otherwise if you can get rid of dataclasses and attribute-annotations (and replace the test class with a standard class with annotations in the init method), then you can move it to _test_py35.py. Once you've done this, please add an appropriate pytest skip mark on your test, and import the module inside the test - similar to https://github.com/smarie/python-makefun/blob/de9caee62cdd2835185ae162173e43da7438b956/tests/test_advanced.py#L222-L233
This should make the build pass.
This should be ready for another test suite run and final review. Thanks for the very speedy review!
Whoops, I missed the instructions in the README about how to actually run the full test suite. I'm working on fixing the test collection bug in 2.7.
Note: the test doesn't work on python <3.7.6, because the __wrapped__ dereferencing in get_type_hints was not added until then. See https://github.com/python/cpython/blob/v3.7.6/Lib/typing.py#L986-L987.
OK, should be passing on all supported versions of python. Thank you for your patience.
nox > Ran multiple sessions:
nox > * tests-3.9: success
nox > * tests-3.8: success
nox > * tests-2.7: success
nox > * tests-3.5: success
nox > * tests-3.6: success
nox > * tests-3.7: success
nox > * flake8: success
I noticed that 3.10 is now available on conda. The tests pass on 3.10 as well. You may want to add 3.10 to the build.
Thanks @lucaswiman !
Sorry to be picky, but I just realized one important thing (see comment in code). Also could you please include in the PR an update to the doc/api_reference.md with the same docstring mod than the ones you did in the code ? Indeed this is not an automated operation at the moment. If I'm not mistaken you just modified the docstring from wraps, but you should probably also modify the docstring from create_function.
Thanks again, almost there :)
@smarie I believe I addressed your review comments. Please let me know if those changes were insufficient or if you need any other updates. Thanks very much!
No worries. Thanks very much!