Skip to content

Commit

Permalink
Revert use of ParamSpec for functools.wraps
Browse files Browse the repository at this point in the history
  • Loading branch information
@AlexWaygood @hauntsaninja
AlexWaygood authored and hauntsaninja committed Sep 16, 2023
1 parent 2816b97 commit 7d987a1
Showing 1 changed file with 14 additions and 26 deletions.
40 changes: 14 additions & 26 deletions mypy/typeshed/stdlib/functools.pyi
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import sys
import types
from _typeshed import SupportsAllComparisons, SupportsItems
from _typeshed import IdentityFunction, SupportsAllComparisons, SupportsItems
from collections.abc import Callable, Hashable, Iterable, Sequence, Sized
from typing import Any, Generic, NamedTuple, TypeVar, overload
from typing_extensions import Literal, ParamSpec, Self, TypeAlias, TypedDict, final
from typing_extensions import Literal, Self, TypeAlias, TypedDict, final

if sys.version_info >= (3, 9):
from types import GenericAlias
Expand All @@ -28,12 +28,10 @@ if sys.version_info >= (3, 8):
if sys.version_info >= (3, 9):
__all__ += ["cache"]

_AnyCallable: TypeAlias = Callable[..., object]

_T = TypeVar("_T")
_S = TypeVar("_S")
_PWrapped = ParamSpec("_PWrapped")
_RWrapped = TypeVar("_RWrapped")
_PWrapper = ParamSpec("_PWrapper")
_RWrapper = TypeVar("_RWrapper")

@overload
def reduce(function: Callable[[_T, _S], _T], sequence: Iterable[_S], initial: _T) -> _T: ...
Expand Down Expand Up @@ -87,41 +85,31 @@ else:
]
WRAPPER_UPDATES: tuple[Literal["__dict__"]]

class _Wrapped(Generic[_PWrapped, _RWrapped, _PWrapper, _RWrapper]):
__wrapped__: Callable[_PWrapped, _RWrapped]
def __call__(self, *args: _PWrapper.args, **kwargs: _PWrapper.kwargs) -> _RWrapper: ...
# as with ``Callable``, we'll assume that these attributes exist
__name__: str
__qualname__: str

class _Wrapper(Generic[_PWrapped, _RWrapped]):
def __call__(self, f: Callable[_PWrapper, _RWrapper]) -> _Wrapped[_PWrapped, _RWrapped, _PWrapper, _RWrapper]: ...

if sys.version_info >= (3, 12):
def update_wrapper(
wrapper: Callable[_PWrapper, _RWrapper],
wrapped: Callable[_PWrapped, _RWrapped],
wrapper: _T,
wrapped: _AnyCallable,
assigned: Sequence[str] = ("__module__", "__name__", "__qualname__", "__doc__", "__annotations__", "__type_params__"),
updated: Sequence[str] = ("__dict__",),
) -> _Wrapped[_PWrapped, _RWrapped, _PWrapper, _RWrapper]: ...
) -> _T: ...
def wraps(
wrapped: Callable[_PWrapped, _RWrapped],
wrapped: _AnyCallable,
assigned: Sequence[str] = ("__module__", "__name__", "__qualname__", "__doc__", "__annotations__", "__type_params__"),
updated: Sequence[str] = ("__dict__",),
) -> _Wrapper[_PWrapped, _RWrapped]: ...
) -> IdentityFunction: ...

else:
def update_wrapper(
wrapper: Callable[_PWrapper, _RWrapper],
wrapped: Callable[_PWrapped, _RWrapped],
wrapper: _T,
wrapped: _AnyCallable,
assigned: Sequence[str] = ("__module__", "__name__", "__qualname__", "__doc__", "__annotations__"),
updated: Sequence[str] = ("__dict__",),
) -> _Wrapped[_PWrapped, _RWrapped, _PWrapper, _RWrapper]: ...
) -> _T: ...
def wraps(
wrapped: Callable[_PWrapped, _RWrapped],
wrapped: _AnyCallable,
assigned: Sequence[str] = ("__module__", "__name__", "__qualname__", "__doc__", "__annotations__"),
updated: Sequence[str] = ("__dict__",),
) -> _Wrapper[_PWrapped, _RWrapped]: ...
) -> IdentityFunction: ...

def total_ordering(cls: type[_T]) -> type[_T]: ...
def cmp_to_key(mycmp: Callable[[_T, _T], int]) -> Callable[[_T], SupportsAllComparisons]: ...
Expand Down

5 comments on commit 7d987a1

@ilevkivskyi
Copy link
Member

Choose a reason for hiding this comment

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

@hauntsaninja @AlexWaygood Is this commit still needed for typeshed syncs? What was the original problem that made us add this? I made a bunch of ParamSpec fixes recently, may be this was fixed as well? If not, I am curious what is the repro, maybe this is something I can fix as well?

@JelleZijlstra
Copy link
Member

Choose a reason for hiding this comment

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

@ilevkivskyi I think this was the problem: python/typeshed#6670 (comment). More discussion in #14815. I haven't looked at the hits in detail, but it seems worth undoing this change and seeing what mypy-primer says.

@AlexWaygood
Copy link
Member Author

Choose a reason for hiding this comment

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

@ilevkivskyi as it happens, I experimented just the other day on my local fork to see what happens if we stop reverting this fix: AlexWaygood#11.

I didn't look too deeply at the primer output on that PR, but it looks to me like it's probably still worth it to continue reverting this change for now. The negative impact of using ParamSpec is now much less than it was when it was originally made to typeshed (python/typeshed#6670 (comment)). But it still looks to me like using ParamSpec might cause some issues for mypy users.

@AlexWaygood
Copy link
Member Author

Choose a reason for hiding this comment

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

Here was the original rationale for adding this commit to the ones we revert whenever we sync typeshed, by the way: #14815 (comment)

@tamird
Copy link
Contributor

@tamird tamird commented on 7d987a1 Jun 11, 2024

Choose a reason for hiding this comment

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

FYI this was finally undone in a00fcba.

Please sign in to comment.