Skip to content

Incorrect rendering of __replace__ method in copy docs: consider adding copy.SupportsReplace? #109961

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

Closed
sobolevn opened this issue Sep 27, 2023 · 10 comments
Assignees
Labels
docs Documentation in the Doc dir topic-typing type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Sep 27, 2023

Bug report

Here's how it looks right now:
Снимок экрана 2023-09-27 в 12 51 08
Link: https://docs.python.org/dev/library/copy.html

But, there's no such thing as copy.__replace__.

There are several options:

  1. Do not document this as a method (however, it is)
  2. Document it as object.__replace__, but object does not have this method - it can be very confusing
  3. Add some protocol, like typing.SupportsReplace and document SupportsReplace.__replace__

I personally think that typing.SupportsReplace is very useful on its own, because:

  • we will use it in typeshed for typing copy.replace
  • it can be used in other places by end users
  • it matches exactly what __replace__ is: a protocol

So, it can something like:

class SupportsReplace(Protocol):
    def __replace__(self, /, **kwargs: Any) -> Self: ...

We cannot really type **kwargs here. See https://discuss.python.org/t/generalize-replace-function/28511/20 But, we can later apply mypy plugin similar to one we use for dataclasses.replace: https://github.com/python/mypy/blob/4b66fa9de07828621fee1d53abd533f3903e570a/mypy/plugins/dataclasses.py#L390-L402

I would like to work on docs / implementation changes after we discuss all the things and come to an agreement :)

CC @serhiy-storchaka @AlexWaygood

Linked PRs

@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error docs Documentation in the Doc dir topic-typing labels Sep 27, 2023
@sobolevn sobolevn self-assigned this Sep 27, 2023
@sobolevn sobolevn changed the title Incorrect rendering of __replace__ method in copy docs Incorrect rendering of __replace__ method in copy docs: consider adding typing.SupportsReplace? Sep 27, 2023
@AA-Turner
Copy link
Member

We should document this in the datamodel, similar to __buffer__, which is also only applicable in certain cases.

A

@AlexWaygood
Copy link
Member

I like the idea of SupportsReplace. I think it should be added to the copy module, though, not typing, similar to how PathLike is in the os module and AbstractContextManager is in contextlib.

We can take the same approach we do with os.PathLike and contextlib.AbstractContextManager in order to avoid importing typing in those modules: have it be an abc.ABC at runtime, but special-case it in typing so that it behaves like a protocol (and pretend in typeshed that it is a protocol). That gives us the best of both worlds.

@sobolevn
Copy link
Member Author

Yes, I agree with copy module. This way it would be also more convenient for users:

from copy import replace, SupportsReplace

@AA-Turner
Copy link
Member

Minor, but perhaps consider Replaceable? That's more in line with the collections ABCs.

A

@AlexWaygood AlexWaygood changed the title Incorrect rendering of __replace__ method in copy docs: consider adding typing.SupportsReplace? Incorrect rendering of __replace__ method in copy docs: consider adding copy.SupportsReplace? Sep 27, 2023
@AlexWaygood
Copy link
Member

Minor, but perhaps consider Replaceable? That's more in line with the collections ABCs.

I think I prefer Nikita's name, which is inline with the Supports* naming convention in the typing module -- SupportsInt, SupportsFloat, etc. "Replaceable" feels like you're making some comment about the fungibility of the object in question, which isn't necessarily true ;)

@AA-Turner
Copy link
Member

AA-Turner commented Sep 27, 2023

Fair enough -- the suggestion was in large part as currently it is only the typing module which uses the SupportsDunder convention. Outwith typing, the following usages appear:

  • typing.Protocol subclasses. All of the following are 'complex' protocols with multiple attributes, save for _Readable, which only defines the read() method.
    • importlib.metadata.{PackageMetadata,SimplePath}
    • importlib.resources.abc.Traversable
    • wsgiref.types.{StartResponse,InputStream,ErrorStream,_Readable,FileWrapper}
  • abc.ABC subclasses or abc.ABCMeta subclasses. Contrary to my earlier note, it seems only 6 of the collections.abc types are named -ble. However, these 6 form a plurality of the 'single method' types (rounding out the set are Buffer, Container, Sized, PathLike, and various number types.
    • collections.abc.{Awaitable,AsyncIterable,Hashable,Iterable,Reversible,Callable }
    • collections.abc.{AsyncGenerator,AsyncIterator,Buffer,ByteString,Collection,Container,Coroutine,Generator,ItemsView,Iterator,KeysView,Mapping,MappingView,MutableMapping,MutableSequence,MutableSet,Sequence,Set,Sized,ValuesView}
    • contextlib.{AbstractContextManager,AbstractAsyncContextManager}
    • importlib.resources.simple.SimpleReader
    • multiprocessing.reduction.AbstractReducer
    • numbers.Number (& by extension the whole numeric tower)
    • os.PathLike
    • selectors.BaseSelector

So it seems consistency is not quite here! I think I sympathise most with your fungability argument, given that this replaces data within the object, not the object itself--though for immutable types the object will be replaced!

A

@AlexWaygood
Copy link
Member

AlexWaygood commented Sep 27, 2023

Yes: objects with the __replace__ method may be fungible, but aren't necessarily fungible :)

@serhiy-storchaka
Copy link
Member

Thank you for catching this @sobolevn.

I think adding object. would be enough. There are no SupportCopy, SupportDeepCopy or SupportPickle. While currently copy.replace() only supports classes with __replace__ method, in future it may support other objects. For example it could support pure Python classes without __new__ and __init__ methods, or some pickleable classes. For now it requires the __replace__ method, but after it became widely used we can analyze patterns of its use and make defining trivial __replace__ methods unneeded.

@AlexWaygood
Copy link
Member

While currently copy.replace() only supports classes with __replace__ method, in future it may support other objects. For example it could support pure Python classes without __new__ and __init__ methods, or some pickleable classes. For now it requires the __replace__ method, but after it became widely used we can analyze patterns of its use and make defining trivial __replace__ methods unneeded.

Ah, if that's the intention, then adding SupportsReplace would indeed be a bad idea.

@serhiy-storchaka
Copy link
Member

Can it be closed already?

@sobolevn sobolevn closed this as completed Oct 7, 2023
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
hugovk pushed a commit to hugovk/cpython that referenced this issue Mar 6, 2025
…` in `copy.rst` (pythonGH-109968)

(cherry picked from commit 0baf726)

Co-authored-by: Nikita Sobolev <[email protected]>
hugovk added a commit that referenced this issue Mar 17, 2025
…copy.rst` (GH-109968) (#130909)

(cherry picked from commit 0baf726)

Co-authored-by: Nikita Sobolev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir topic-typing type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants