Skip to content

Bugfix/issue 555 play nice with other decorator - #666

Closed
nhtua wants to merge 17 commits into
pyinvoke:mainfrom
nhtua:bugfix/issue-555-play-nice-with-other-decorator
Closed

nhtua wants to merge 17 commits into
pyinvoke:mainfrom
nhtua:bugfix/issue-555-play-nice-with-other-decorator

Conversation

@nhtua

@nhtua nhtua commented Sep 13, 2019 •

Copy link
Copy Markdown

I made these changes to implement for:
#246
#399
#555

It's more than 4 years from the date when people found it hard to apply to Invoke @task on other @decorator. The root cause of it is inspect.getargspec which couldn't handle decorated function. It's also recommended replacing by inspect.signature for a "more structured introspection API for callables" from python 3.4.

I also keep the compatibility with Python 2.x by using the original function under the name argspec_python2. New argspec will check if it is Python 2 and switch the flow to argspec_python2 on runtime.

@nhtua

nhtua commented Sep 13, 2019 •

Copy link
Copy Markdown
Author

Don't know if we should drop support for Python 2.7 now. It's up to you bitprophet.
I've updated my PR to be compatible backward to Python 2.7
But I intend to support @task applying on top of others decorator only on Python 3 because Python 2.x does not have inspect.signature.

@nhtua

nhtua commented Sep 17, 2019

Copy link
Copy Markdown
Author

I will update some test-cases to ensure argspec() will work well with multi decorators.

amatissart added a commit to Qwant/kartotherian_docker that referenced this pull request Dec 3, 2019
This is caused by a known limitation in invoke: pyinvoke/invoke#555
See pyinvoke/invoke#666 for a possible future fix
@nhtua
nhtua force-pushed the bugfix/issue-555-play-nice-with-other-decorator branch from d71a348 to 139f5e8 Compare January 20, 2020 06:09
@nhtua

nhtua commented Jan 20, 2020 •

Copy link
Copy Markdown
Author

I've added unit test to cover cases of applying multi decorators (both function and class decorators).

@nhtua
nhtua requested review from bitprophet and removed request for bitprophet January 20, 2020 08:24
@nhtua

nhtua commented Jan 20, 2020

Copy link
Copy Markdown
Author

Sorry, Just accidentally clicked the button Request Review. I cancelled the request. Don't want to be rude to ask you review it now. Take your time. Thank you!

@lyrixx

lyrixx commented Mar 13, 2020

Copy link
Copy Markdown

Hello @bitprophet, did you see this PR ? I would be fantastic to have support for decorator 😍

@phelian

phelian commented Jan 26, 2021

Copy link
Copy Markdown

Another voice for getting this in @bitprophet

@hjoukl

hjoukl commented Sep 14, 2021

Copy link
Copy Markdown

I'd also like to see this go in :-).

FWIW if Python 2.7 compatibility is of prime concern here one could still fall back to use inspect.getargspec(f.__wrapped__) for the Python 2.7 case (as you probably know). Provided you can use a modified/enhanced alternative to functools.wraps for the decoration, as Py2-functools.wraps does not set this convenient attribute.

Some helpful discussion (imho) can be found here: https://stackoverflow.com/a/43506509/7379507

@neozenith neozenith self-assigned this Mar 21, 2022
@neozenith

Copy link
Copy Markdown

Looks like this overlaps with some other issues reported about type checking in #606 and #458
I'll have a look at reviewing this area of the code over coming weeks especially if we can get a 2-for-1

@bitprophet

Copy link
Copy Markdown
Member

2.0.0 will be out soon & is now using inspect.signature!

@bitprophet bitprophet closed this Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants