typeshed icon indicating copy to clipboard operation
typeshed copied to clipboard

Make `copy.replace` more strongly typed

Open decorator-factory opened this issue 3 months ago • 8 comments

The current stub for copy.replace accepts arbitrary keyword arguments. We can use ParamSpec to link the signature of obj.__replace__ to the supplied keyword arguments.

This allows passing positional arguments to copy.replace, but I think it's fine, given that __replace__ isn't supposed to contain positional arguments in the signature anyway (docs for object.__replace__)

Demo of the proposed stub for copy.replace in the pyright playground:

from dataclasses import dataclass
from typing import Protocol

import copy

@dataclass(frozen=True)
class Fruit:
  name: str
  long: bool

banana = Fruit("Banana", True)
reveal_type(banana.__replace__)  # (*, name: str = ..., long: bool = ...) -> Fruit

class _SupportsReplace[**P, T](Protocol):
    def __replace__(self, /, *args: P.args, **kwargs: P.kwargs) -> T:
        ...

def replace_v2[**P, T](obj: _SupportsReplace[P, T], *args: P.args, **kwargs: P.kwargs) -> T:
  return obj.__replace__(*args, **kwargs)

# using the current copy.replace stub
x = copy.replace(banana, name="Apple")              # no error, as expected
y = copy.replace(banana, name="Apple", long=False)  # no error, as expected
z = copy.replace(banana, missing=42)                # no error, but should be
w = copy.replace(banana, name="Apple", long=42)     # no error, but should be

# using suggested change
x = replace_v2(banana, name="Apple")              # no error, as expected
y = replace_v2(banana, name="Apple", long=False)  # no error, as expected
z = replace_v2(banana, missing=42)                # error: no parameter named missing, as expected
w = replace_v2(banana, name="Apple", long=42)     # error: wrong type, as expected

decorator-factory avatar Oct 01 '25 23:10 decorator-factory

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Oct 01 '25 23:10 github-actions[bot]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Oct 02 '25 00:10 github-actions[bot]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Oct 02 '25 00:10 github-actions[bot]

This allows passing positional arguments to copy.replace, but I think it's fine, given that __replace__ isn't supposed to contain positional arguments in the signature anyway

LSP-wise, a __replace__ with positional- or keyword-only arguments is allowed (i.e. no *,). So in those cases this will introduce a new false negative when calling copy.replace with *args. But on the other hand, this also removes a false negative in the **kwargs. And if that's something that generally occurs more often, then this change will reduce the expected number of bugs. My gut feeling says that's indeed more likely, but I can also imagine someone mistakenly passing *args to replace in case of a tuple or something. Perhaps vibe-coding a small code survey could help here.

jorenham avatar Oct 06 '25 03:10 jorenham

@jorenham Yes, unfortunately there's a tradeoff here :frowning_face:

But on the other hand, this also removes a false negative in the **kwargs. And if that's something that generally occurs more often, then this change will reduce the expected number of bugs.

Pyright currently synthesizes a __replace__ method for dataclasses (including decorators/metaclasses using the @dataclass_transform decorator, like attrs and pydantic models). I'm thinking that a lot of copy.replace calls are going to go to these synthesized methods, which only have keyword arguments.

Another consideration: if you call copy.replace with positional arguments, it's at least going to fail at runtime. If you replace an item with a value of the wrong type:

@dataclass
class Fruit:
    name: str
    
apple = Fruit("apple")
banana = copy.replace(apple, name=None)

it's not going to complain. So accepting positional arguments while catching kwargs with incorrect types can find bugs that don't cause an immediate runtime error.


I'm not sure if a good survey can be taken since copy.replace is pretty new, but as a singular data point, this change found a typo in the typeshed tests https://github.com/python/typeshed/pull/14819/files#diff-57c0ce5b9482eae15b859f6a5ab6e89254e5554fcc350f68f6985660e43a0706R32-R39

decorator-factory avatar Oct 06 '25 05:10 decorator-factory

Hack suggestion: add an optional positional argument with an unsatisfiable type.

    def replace(
        obj: _SupportsReplace[_P, _RT_co],
        _hack: Never = ...,  # HACK: disallow positional arguments
        /, *_: _P.args, **changes: _P.kwargs,
    ) -> _RT_co: ...

With this change, calling replace with any positional arguments after obj will cause a type checking error, as the first argument would have to match Never. There's an annoying exception that a value of type Any is assignable to Never (or at least mypy and pyright think that it is).

Downsides: the error and the signature will have a confusing _hack argument.

decorator-factory avatar Oct 06 '25 05:10 decorator-factory

Hack suggestion: add an optional positional argument with an unsatisfiable type.

    def replace(
        obj: _SupportsReplace[_P, _RT_co],
        _hack: Never = ...,  # HACK: disallow positional arguments
        /, *_: _P.args, **changes: _P.kwargs,
    ) -> _RT_co: ...

With this change, calling replace with any positional arguments after obj will cause a type checking error, as the first argument would have to match Never. There's an annoying exception that a value of type Any is assignable to Never (or at least mypy and pyright think that it is).

Downsides: the error and the signature will have a confusing _hack argument.

That's a pretty awesome hack, IMO. But let's see if the maintainers agree.

jorenham avatar Oct 15 '25 05:10 jorenham

Personally, I think the argument that __replace__ implementations will overwhelmingly have , *, in the signature is pretty convincing. And if a __replace__ implementation doesn't have it and someone passes in an argument positionally, then they'll get an error and can fix the __replace__ implementation.

In any case, the current situation seems worse, where copy.replace() just accepts anything.

The topic has also come up on DPO: https://discuss.python.org/t/better-typing-for-copy-replace-functions-that-operate-on-dataclasses/105004

tmke8 avatar Nov 24 '25 16:11 tmke8