Make `copy.replace` more strongly typed
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
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉
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 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
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.
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
replacewith any positional arguments afterobjwill cause a type checking error, as the first argument would have to matchNever. There's an annoying exception that a value of typeAnyis assignable toNever(or at least mypy and pyright think that it is).Downsides: the error and the signature will have a confusing
_hackargument.
That's a pretty awesome hack, IMO. But let's see if the maintainers agree.
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