Skip to content

Add copy.replace for 3.13 #12262

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

Merged
merged 24 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions stdlib/@tests/test_cases/check_copy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from __future__ import annotations

import copy
import sys
from typing_extensions import Self, assert_type


class ReplaceableClass:
def __init__(self, val: int) -> None:
self.val = val

def __replace__(self, val: int) -> Self:
cpy = copy.copy(self)
cpy.val = val
return cpy


if sys.version_info >= (3, 13):
obj = ReplaceableClass(42)
cpy = copy.replace(obj, val=23)
assert_type(cpy, ReplaceableClass)
14 changes: 13 additions & 1 deletion stdlib/copy.pyi
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
from typing import Any, TypeVar
import sys
from typing import Any, Protocol, TypeVar
from typing_extensions import ParamSpec, Self

__all__ = ["Error", "copy", "deepcopy"]

_T = TypeVar("_T")
_SR = TypeVar("_SR", bound=_SupportsReplace[Any])
_P = ParamSpec("_P")

class _SupportsReplace(Protocol[_P]):
# In reality doesn't support args, but there's no other great way to express this.
def __replace__(self, *args: _P.args, **kwargs: _P.kwargs) -> Self: ...
Copy link
Member

Choose a reason for hiding this comment

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

Technically speaking, it does support positionals, but those will be passed as keywords. What it does not support however is positional-only arguments.

On the other hand, you can just force people to use keyword-only arguments so that it matches https://docs.python.org/3.13/library/copy.html#object.__replace__. Or at least, make self a positional-only argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes, callables with *args: Any, **kwargs: Any are treated specially in the spec.


# None in CPython but non-None in Jython
PyStringMap: Any
Expand All @@ -11,6 +19,10 @@ PyStringMap: Any
def deepcopy(x: _T, memo: dict[int, Any] | None = None, _nil: Any = []) -> _T: ...
def copy(x: _T) -> _T: ...

if sys.version_info >= (3, 13):
__all__ += ["replace"]
def replace(obj: _SR, /, **changes: Any) -> _SR: ...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See us mostly using += in most places with __all__, guessing this is for better static analysis support, or something like that?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is fine, it's so we don't have to repeat the whole list for every version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I meant versus using append.

Copy link
Member

Choose a reason for hiding this comment

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

Except it actually isn't in __all__: https://github.com/python/cpython/blob/89bf33732fd53fbb954aa088cfb34f13264746ad/Lib/copy.py#L59 . So we shouldn't add it in the stub either.

This may be an oversight, so consider opening an issue over on CPython for adding replace to copy.__all__. But as long as it's not there at runtime, we shouldn't add it in the stub either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, let me fix that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


class Error(Exception): ...

error = Error
Loading