Skip to content

Add type stub for decorator lib#3038

Merged
srittau merged 2 commits intopython:masterfrom
RafiB:add-decorator-stub
Oct 7, 2019
Merged

Add type stub for decorator lib#3038
srittau merged 2 commits intopython:masterfrom
RafiB:add-decorator-stub

Conversation

@RafiB
Copy link
Contributor

@RafiB RafiB commented Jun 7, 2019

I emailed the repo owner directly and got permission to add this stub; here's a public issue for visibility/posterity micheles/decorator#69

I tested some of this behaviour locally; I'll be adding a mypy test in a separate pull request to the mypy repo.
For the purpose of code-review, here's a test I used:

    1 from typing import Any, Callable, Text, TypeVar
    2 from decorator import decorate, decorator
    3
    4
    5 MYPY = False
    6
    7 F = TypeVar('F', bound=Callable[..., Any])
    8
    9 @decorator
   10 def print_foo_before(func, *args, **kwargs):
   11     # type: (F, *Any, **Any) -> None
   12     print('foo')
   13     func(*args, **kwargs)
   14
   15 @print_foo_before
   16 def greet(message):
   17     # type: (Text) -> Text
   18     print(message)
   19     return message
   20
   21 greet('bar')
   22 greet(1)
   23
   24 def _memoize(func, *args, **kw):
   25     # type: (F, *Any, **Any) -> F
   26     return func(*args, **kw)
   27
   28 def memoize(f):
   29     # type: (F) -> F
   30     """
   31     A simple memoize implementation. It works by adding a .cache dictionary
   32     to the decorated function. The cache will grow indefinitely, so it is
   33     your responsibility to clear it, if needed.
   34     """
   35     f.cache = {}  # type: ignore
   36     decorated = decorate(f, _memoize)
   37     if MYPY:
   38         reveal_type(decorated)
   39     return decorated
   40
   41 if MYPY:
   42     reveal_type(greet)
   43     reveal_type(memoize(greet))
   44
   45 from decorator import getfullargspec
   46 if MYPY:
   47     reveal_type(getfullargspec)

running the test generates

$ python test.py
foo
bar
foo
1

running mypy generates

$ mypy test.py --custom-typeshed-dir=/Users/rafblecher/typeshed-dev --disallow-untyped-decorators
test.py:22: error: Argument 1 to "greet" has incompatible type "int"; expected "str"
test.py:38: error: Revealed type is 'F`-1'
test.py:42: error: Revealed type is 'def (message: builtins.str) -> builtins.str'
test.py:43: error: Revealed type is 'def (message: builtins.str) -> builtins.str'
test.py:47: error: Revealed type is 'def (builtins.object) -> Tuple[builtins.list[builtins.str], Union[builtins.str, None], Union[builtins.str, None], builtins.tuple[Any], builtins.list[builtins.str], builtins.dict[builtins.str, Any], builtins.dict[builtins.str, Any], fallback=decorator.FullArgSpec]'

Line 23 demonstrates that the type annotation of greet is correct after decoration with @print_before; trying to pass 1 as Text causes a type error.
Similarly, to give more confidence, lines 38-43 show that decorated matches the type signature of f so that memoize fulfills its contract of (F) -> F; resulting in greet and memoize(greet) having the same type.

Depends on #3083

@srittau
Copy link
Collaborator

srittau commented Jun 13, 2019

Thank you for your pull request. Could you break out the change to contextlib into a separate PR, for easier synchronization with the mypy team?

@ilevkivskyi
Copy link
Member

Yes, splitting the the PR in two parts will definitely simplify the sync.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already mentioned splitting out the changes to contextlib. I left further remarks below.

RafiB pushed a commit to RafiB/mypy that referenced this pull request Jun 22, 2019
RafiB pushed a commit to RafiB/mypy that referenced this pull request Jun 22, 2019
RafiB pushed a commit to RafiB/mypy that referenced this pull request Jun 22, 2019
@RafiB RafiB force-pushed the add-decorator-stub branch from ccd30a9 to e3e10cd Compare June 22, 2019 05:25
RafiB pushed a commit to RafiB/mypy that referenced this pull request Jun 22, 2019
@RafiB RafiB force-pushed the add-decorator-stub branch 2 times, most recently from 66e8489 to 5b44391 Compare June 22, 2019 05:33
@RafiB
Copy link
Contributor Author

RafiB commented Jun 22, 2019

I split the contextlib changes out into #3083

@ilevkivskyi
Copy link
Member

OK, all tests now pass here.

@JelleZijlstra
Copy link
Member

@srittau could you review this again?

@RafiB
Copy link
Contributor Author

RafiB commented Oct 6, 2019

bump, can someone look at this again please? :)

@srittau
Copy link
Collaborator

srittau commented Oct 6, 2019

Sorry for missing this. I will look at it tomorrow.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry again for forgetting about this. This is a full re-review, so I might remark on things that I have remarked on before.

@RafiB RafiB force-pushed the add-decorator-stub branch 2 times, most recently from 7f28422 to 9f96b73 Compare October 7, 2019 19:47
@RafiB RafiB force-pushed the add-decorator-stub branch from 9f96b73 to 8fcc03f Compare October 7, 2019 19:52
@srittau
Copy link
Collaborator

srittau commented Oct 7, 2019

For future reference: please don't force push, otherwise it is hard for reviewers to see what has changed since the last review.

@srittau srittau merged commit 6e4708e into python:master Oct 7, 2019
@RafiB RafiB deleted the add-decorator-stub branch October 7, 2019 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants