Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

please help how to correctly type annotate a wrapt decorator .... #164

Open
bitranox opened this issue Jul 10, 2020 · 14 comments
Open

please help how to correctly type annotate a wrapt decorator .... #164

bitranox opened this issue Jul 10, 2020 · 14 comments

Comments

@bitranox
Copy link

Dear Graham,
I have a simple decorator, which checks if Keywords are present -

It is a PEP561 Package, so type annotated - but I cant annotate for wrapt :

I tried a lot of different versions, but mypy doesnt stop to complain.

F = TypeVar('F', bound=Callable[..., Any])


@wrapt.decorator
def _check_for_kwargs(wrapped: F, instance: object, args: Any, kwargs: Any) -> F:
    if kwargs:  # pragma: no cover
        keys = ', '.join([key for key in kwargs.keys()])  # pragma: no cover
        raise TypeError("{fn}() got some positional-only arguments passed as keyword arguments: '{keys}'".format(
            fn=wrapped.__name__, keys=keys))  # pragma: no cover
    return cast(F, wrapped(*args, **kwargs))


fake_winreg.py:22: error: Untyped decorator makes function "_check_for_kwargs" untyped
fake_winreg.py:96: error: Untyped decorator makes function "CloseKey" untyped
fake_winreg.py:122: error: Untyped decorator makes function "ConnectRegistry" untyped
...

please help !

Robert

@GrahamDumpleton
Copy link
Owner

I don't know how to use type annotations, so don't really understand what you are trying to do and have no immediate idea. Can you expand more on how it is meant to all work and what for?

May or may not be relevant, but also look at using an adapter prototype function if need to have a different signature/annotations for a target function be exported.

It is also quite possible that the function wrapper in wrapt isn't passing back annotations information correct since was written before they were introduced and never used them. Is there more than __annotations__ that needs to be proxied.

@bitranox
Copy link
Author

Dear Graham,

I try to archive that mypy passes - but it does not.

The decorator itself works without Problems.
Since I want to be python 3.6 compatible, I can not use the "positional only" syntax (PEP570).

The whole thing is to to mock a C Librarary (winreg) that only accepts positional arguments.

Since it seems to be impossible within a function to find out if a parameter was passed as argument or as
keyword argument without changing the signature to (*args, **kwargs), I need to use a decorator.

I guess that this type annotation has to happen in wrapt, because You do a lot of magic there,
so the mypy inspector freaks out.

In general I have the opinion its totally worth to use type annotations especially on bigger projects,
You might read https://dropbox.tech/application/our-journey-to-type-checking-4-million-lines-of-python

MAYBE I can solve it by making a stub file for the wrapt decorator, I will try it after my second coffee.

All the best and thanks for Your great work and fast reply.

Robert, Vienna

@bitranox
Copy link
Author

bitranox commented Jul 11, 2020

Dear Graham,
here my findings :

You can type annotate a third party package with stub files, which I did for the decorator function of wrapt.

  • put a wrapt.pyi stub file in .../project/stubs folder, and add that folder to the mypypath :
# .../project/package/stubs/wrapt.pyi

from typing import Any, TypeVar, Callable, Optional

F = TypeVar('F', bound=Callable[..., Any])    # type of the wrapped function (? or the wrapper, I am confused)

A = TypeVar('A', bound=Callable[..., Any])   # type of the adapter function

def decorator(wrapper: F, enabled: Optional[bool] = None, adapter: Optional[A] = None) -> F: ...

# working example that passes mypy
from typing import Any, Callable, Dict, Optional, Tuple, TypeVar, Union, cast

F = TypeVar('F', bound=Callable[..., Any])


@wrapt.decorator
def check_for_kwargs_wrapt(wrapped: F, instance: object = None, args: Any = (), kwargs: Any = dict()) -> F:     # noqa
    if kwargs:                                            # pragma: no cover
        keys = ', '.join([key for key in kwargs.keys()])  # pragma: no cover
        raise TypeError("{fn}() got some positional-only arguments passed as keyword arguments: '{keys}'".format(
            fn=wrapped.__name__, keys=keys))              # pragma: no cover
    return cast(F, wrapped(*args, **kwargs))


@check_for_kwargs_wrapt
def test(x: int) -> None:
    """
    >>> test(x=5)
    Traceback (most recent call last):
        ...
    TypeError: test() got some positional-only arguments passed as keyword arguments: 'x'

    """
    print(x)

mypy can pass now
The pycharm warning about the wrapt decorators also goes away,
as soon as I put the default values ... instance: object = None, args: Any = (), ...

(I read in the wrapt issues some users asking for that)

It would be really cool if You consider to make wrapt a PEP561 package, in order to be able to use mypy with it,
since type annotation (and justifiably so) gets more and more adopted.

I guess, it would be enough to use my suggested wrapt.pyi, put it together with an empty py.typed file in the wrapt package folder, include those files in setup packagedata and we are good to go.

You dont need to fully annotate everything, wrapt.decorator would be a good starter .

And if something happens not to be correct, users still can use the # type: ignore marker to ignore glitches - so You would not brake anything at runtime.

This would make wrapt even more useful for me, though I found a solution for my purpose.

Its not nice to take care for the 3rd party stubs, include the paths in the mypy path, etc - so it would be wonderful if that works out of the box.

hoo roo

Robert, Vienna

@bitranox bitranox changed the title please help how to correctly type annotate a warapt decorator .... please help how to correctly type annotate a wrapt decorator .... Jul 11, 2020
@bdsoha
Copy link

bdsoha commented Dec 29, 2022

@bitranox Any headway on the ObjectProxy types?

I seemed to make mypy happy with the following wrapt.pyi:

from typing import Any, Generic, TypeVar


T = TypeVar("T", bound=Any)

class ObjectProxy(Generic[T]): 
    __wrapped__ : T

    def __init__(self, wrapped: T):
        ...

@GrahamDumpleton
Copy link
Owner

If any solution requires adding annotation into the original wrapt source code, then it needs to wait until Python 2.7 support is dropped from wrapt.

Besides that, I still don't know enough about annotations to know what changes would be required.

@bitranox
Copy link
Author

Dear Graham,
as stated before - it would be enough to add my suggested "wrapt.pyi", put it together with an empty "py.typed" file in the wrapt package folder, include those files in setup packagedata and we are good to go.
You might add the kind additions of @bdsoha to add typing for class "ObjectProxy".
No further additions to the original sourcecode are needed, and it will not break anything.
yours sincerely
Robert

@bitranox
Copy link
Author

Graham, if You want I can make a PR and You test it ?

@GrahamDumpleton
Copy link
Owner

I am hoping to set aside some time in coming week to catch up on some wrapt stuff, so if you can provide a PR to get me started that would be most helpful.

I didn't previously understand your comment about stub files as I didn't know Python supported such a concept and thought annotations needed to be in the source code.

@bdsoha
Copy link

bdsoha commented Jan 1, 2023

@GrahamDumpleton Regarding your comment on it needs to wait until Python 2.7 support is dropped from wrapt, why can't you publish a separate branch without Python2 support?

@GrahamDumpleton
Copy link
Owner

Would only do that after had dropped Python 2.7 support, and only if need to back port critical fixes. I don't get time to work on it as it is so don't want to create extra work for myself if don't need to. :-)

@bitranox
Copy link
Author

bitranox commented Jan 1, 2023

I dont see how python 2.7 should be affected. The stub files will just be ignored by Python 2.7, so I dont see any problem with it.

@bdsoha
Copy link

bdsoha commented Jan 1, 2023

I dont see how python 2.7 should be affected. The stub files will just be ignored by Python 2.7, so I dont see any problem with it.

I was referring to a more robust typed solution targeting PY3+.

@GrahamDumpleton
Copy link
Owner

Did you by chance get an opportunity to throw together a type hints file to get me started? Have started reading up on how they work but will take me some more time to work out what I need to provide the hints for.

@bitranox
Copy link
Author

I will make a PR tomorrow, should be easy

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

No branches or pull requests

3 participants