Skip to content

Conversation

decorator-factory
Copy link

@decorator-factory decorator-factory commented Oct 1, 2025

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

This comment has been minimized.

This comment has been minimized.

1 similar comment
Copy link
Contributor

github-actions bot commented Oct 2, 2025

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

@jorenham
Copy link
Contributor

jorenham commented Oct 6, 2025

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.

@decorator-factory
Copy link
Author

@jorenham Yes, unfortunately there's a tradeoff here ☹️

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
Copy link
Author

decorator-factory commented Oct 6, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants