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

Use typing.TypeVar on methods used as annotations #1654

Closed
marcoffee opened this issue Sep 29, 2022 · 2 comments · Fixed by #1655
Closed

Use typing.TypeVar on methods used as annotations #1654

marcoffee opened this issue Sep 29, 2022 · 2 comments · Fixed by #1655
Labels
agent-python community Issues opened by the community triage Issues awaiting triage

Comments

@marcoffee
Copy link
Contributor

marcoffee commented Sep 29, 2022

Is your feature request related to a problem? Please describe.
When I annotate a method with, for example, @capture_span, the method loses its type hints and becomes just a Callable.
For example, Pylance gives the following information tooltips (as commented to the right of the reveal_type calls):

import elasticapm as apm
from typing_extensions import reveal_type


def test (a: int, b: str, c: float) -> bytes:
    return b"abc"

@apm.capture_span()
def test_with_apm (a: int, b: str, c: float) -> bytes:
    return b"abc"

reveal_type(test)  # Type of "test" is "(a: int, b: str, c: float) -> bytes"
reveal_type(test_with_apm)  # Type of "test_with_apm" is "(...) -> Any"

Describe the solution you'd like
Just use TypeVar on the __call__ method signature of annotation methods, example:

from typing import TypeVar

_AnnotatedFunctionT = TypeVar("_AnnotatedFunctionT")

class capture_span(object):
    ...
    def __call__(self, func: _AnnotatedFunctionT) -> _AnnotatedFunctionT:
        self.name = self.name or get_name_from_func(func)

        @functools.wraps(func)
        def decorated(*args, **kwds):
            with self:
                return func(*args, **kwds)

        return decorated
    ...

After this small change, the reveal_type reports the expected type:

import elasticapm as apm
from typing_extensions import reveal_type

@apm.capture_span()
def test_with_apm (a: int, b: str, c: float) -> bytes:
    return b"abc"

reveal_type(test_with_apm)  # Type of "test_with_apm" is "(a: int, b: str, c: float) -> bytes"

Describe alternatives you've considered
I've considered creating a pull request myself to solve this, but the CONTRIBUTING.md file indicates that an issue should be opened first.

Additional context
Screenshot of VSCode's pylance type hints without the change:
apm_test_without_typevar

Screenshot of VSCode's pylance type hints with the change:
apm_test_with_typevar

@github-actions github-actions bot added agent-python community Issues opened by the community triage Issues awaiting triage labels Sep 29, 2022
@marcoffee marcoffee changed the title Use typing.TypeVar on function annotations Use typing.TypeVar on methods used as annotations Sep 29, 2022
@beniwohli
Copy link
Contributor

Hey @marcoffee

That's a great improvement! Thanks for proposing this, a PR would be great! Do you think it would also make sense to annotate the actual decorator (the decorated function) with ParamSpec? That is, if we can find a way to do it in a backwards-compatible way, as ParamSpec is only available in Python 3.10...

@marcoffee
Copy link
Contributor Author

marcoffee commented Sep 30, 2022

@beniwohli

Thanks for the positive feedback! I think there is an implementation of the ParamSpec on the typing-extensions package, but it is for python >= 3.7, so compatibility would still be broken for 3.5 and 3.6. Also, there is the issue of adding another requirement.
Anyways, I think there is no need to annotate the internal method, since functools.wrap already solve the issue for runtime, and the TypeVar ensures the typing of the return.
I will start working on the PR as soon as possible.

marcoffee added a commit to marcoffee/apm-agent-python that referenced this issue Sep 30, 2022
beniwohli pushed a commit that referenced this issue Oct 4, 2022
* Fixes issue #1654

Co-authored-by: Colton Myers <colton@myers.fm>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-python community Issues opened by the community triage Issues awaiting triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants