-
-
Notifications
You must be signed in to change notification settings - Fork 477
refactor: ♻️ Use typing_extensions.deprecated
instead of own implementation
#2655
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,6 @@ | |
import asyncio | ||
import collections.abc | ||
import datetime | ||
import functools | ||
import itertools | ||
import json | ||
import re | ||
|
@@ -63,6 +62,8 @@ | |
overload, | ||
) | ||
|
||
from typing_extensions import deprecated as _typing_deprecated | ||
|
||
from .errors import HTTPException, InvalidArgument | ||
|
||
try: | ||
|
@@ -289,20 +290,19 @@ def decorator(overridden: T) -> T: | |
return decorator | ||
|
||
|
||
def warn_deprecated( | ||
def _get_deprecated_message( | ||
name: str, | ||
instead: str | None = None, | ||
since: str | None = None, | ||
removed: str | None = None, | ||
reference: str | None = None, | ||
stacklevel: int = 3, | ||
) -> None: | ||
"""Warn about a deprecated function, with the ability to specify details about the deprecation. Emits a | ||
DeprecationWarning. | ||
) -> str: | ||
""" | ||
Helper function to generate a deprecation message. | ||
|
||
Parameters | ||
---------- | ||
name: str | ||
name: :class:`str` | ||
The name of the deprecated function. | ||
instead: Optional[:class:`str`] | ||
A recommended alternative to the function. | ||
|
@@ -315,10 +315,12 @@ def warn_deprecated( | |
reference: Optional[:class:`str`] | ||
A reference that explains the deprecation, typically a URL to a page such as a changelog entry or a GitHub | ||
issue/PR. | ||
stacklevel: :class:`int` | ||
The stacklevel kwarg passed to :func:`warnings.warn`. Defaults to 3. | ||
|
||
Returns | ||
------- | ||
:class:`str` | ||
The deprecation message. | ||
""" | ||
warnings.simplefilter("always", DeprecationWarning) # turn off filter | ||
message = f"{name} is deprecated" | ||
if since: | ||
message += f" since version {since}" | ||
|
@@ -329,7 +331,40 @@ def warn_deprecated( | |
message += "." | ||
if reference: | ||
message += f" See {reference} for more information." | ||
return message | ||
|
||
|
||
def warn_deprecated( | ||
name: str, | ||
instead: str | None = None, | ||
since: str | None = None, | ||
removed: str | None = None, | ||
reference: str | None = None, | ||
stacklevel: int = 3, | ||
) -> None: | ||
"""Warn about a deprecated function, with the ability to specify details about the deprecation. Emits a | ||
DeprecationWarning. | ||
|
||
Parameters | ||
---------- | ||
name: str | ||
The name of the deprecated function. | ||
instead: Optional[:class:`str`] | ||
A recommended alternative to the function. | ||
since: Optional[:class:`str`] | ||
The version in which the function was deprecated. This should be in the format ``major.minor(.patch)``, where | ||
the patch version is optional. | ||
removed: Optional[:class:`str`] | ||
The version in which the function is planned to be removed. This should be in the format | ||
``major.minor(.patch)``, where the patch version is optional. | ||
reference: Optional[:class:`str`] | ||
A reference that explains the deprecation, typically a URL to a page such as a changelog entry or a GitHub | ||
issue/PR. | ||
stacklevel: :class:`int` | ||
The stacklevel kwarg passed to :func:`warnings.warn`. Defaults to 3. | ||
""" | ||
warnings.simplefilter("always", DeprecationWarning) # turn off filter | ||
message = _get_deprecated_message(name, instead, since, removed, reference) | ||
warnings.warn(message, stacklevel=stacklevel, category=DeprecationWarning) | ||
warnings.simplefilter("default", DeprecationWarning) # reset filter | ||
|
||
|
@@ -339,12 +374,11 @@ def deprecated( | |
since: str | None = None, | ||
removed: str | None = None, | ||
reference: str | None = None, | ||
stacklevel: int = 3, | ||
stacklevel: int = 1, | ||
*, | ||
use_qualname: bool = True, | ||
) -> Callable[[Callable[[P], T]], Callable[[P], T]]: | ||
"""A decorator implementation of :func:`warn_deprecated`. This will automatically call :func:`warn_deprecated` when | ||
the decorated function is called. | ||
) -> _typing_deprecated: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should test if this is actually detected correctly by type checkers There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I forgot to check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So it is not apparently, at least basedpyright. But in general imo this solution in this pr was kinda hacky. The thing is to solve #1917 we either have to give up the custom message thing, or give up it being picked up by type checkers. Additionally, things like |
||
"""A decorator implementation of :func:`typing.deprecated` that allows for more detailed deprecation messages. | ||
|
||
Parameters | ||
---------- | ||
|
@@ -360,27 +394,25 @@ def deprecated( | |
A reference that explains the deprecation, typically a URL to a page such as a changelog entry or a GitHub | ||
issue/PR. | ||
stacklevel: :class:`int` | ||
The stacklevel kwarg passed to :func:`warnings.warn`. Defaults to 3. | ||
The stacklevel kwarg passed to :func:`typing.deprecated`. Defaults to 1. | ||
use_qualname: :class:`bool` | ||
Whether to use the qualified name of the function in the deprecation warning. If ``False``, the short name of | ||
the function will be used instead. For example, __qualname__ will display as ``Client.login`` while __name__ | ||
will display as ``login``. Defaults to ``True``. | ||
""" | ||
|
||
def actual_decorator(func: Callable[[P], T]) -> Callable[[P], T]: | ||
@functools.wraps(func) | ||
def decorated(*args: P.args, **kwargs: P.kwargs) -> T: | ||
warn_deprecated( | ||
name=func.__qualname__ if use_qualname else func.__name__, | ||
instead=instead, | ||
since=since, | ||
removed=removed, | ||
reference=reference, | ||
stacklevel=stacklevel, | ||
) | ||
return func(*args, **kwargs) | ||
|
||
return decorated | ||
def actual_decorator(func: T) -> T: | ||
return _typing_deprecated( | ||
_get_deprecated_message( | ||
func.__qualname__ if use_qualname else func.__name__, | ||
instead, | ||
since, | ||
removed, | ||
reference, | ||
), | ||
stacklevel=stacklevel, # this seems to work | ||
category=DeprecationWarning, | ||
)(func) | ||
|
||
return actual_decorator | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
aiohttp>=3.6.0,<4.0 | ||
typing_extensions>=4,<5 | ||
typing_extensions>=4.5.0,<5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typing's thing for some reason changes the levels in some way and to keep a correct stacktrace I had to change it. Admittedly I just tried and see.