Skip to content
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

Protocol for an instance method taking a self argument #15620

Closed
Kludex opened this issue Jul 7, 2023 · 8 comments · Fixed by pydantic/pydantic#6514
Closed

Protocol for an instance method taking a self argument #15620

Kludex opened this issue Jul 7, 2023 · 8 comments · Fixed by pydantic/pydantic#6514
Labels
bug mypy got something wrong

Comments

@Kludex
Copy link

Kludex commented Jul 7, 2023

Bug Report

We have several protocols we use to define @model_validator in Pydantic V2, see here.

The problem is that the simplest use case fails. See below:

To Reproduce

from pydantic import BaseModel, model_validator


class UserModel(BaseModel):
    username: str
    password1: str
    password2: str

    @model_validator(mode="after")
    def check_passwords_match(self) -> "UserModel":
        if self.password1 != self.password2:
            raise ValueError("passwords do not match")
        return self

Expected Behavior

I expect no issues with mypy. As I don't have any with pyright.

Actual Behavior

a.py:9: error: Argument 1 has incompatible type "Callable[[UserModel], UserModel]"; expected "ModelAfterValidator | ModelAfterValidatorWithoutInfo"  [arg-type]
Found 1 error in 1 file (checked 1 source file)

Your Environment

  • Mypy version used: mypy 1.4.1 (compiled: yes)
  • Mypy command-line flags: none
  • Mypy configuration options from mypy.ini (and other config files): no config file provided.
  • Python version used: 3.11.1
@adriangb
Copy link
Contributor

adriangb commented Jul 7, 2023

Here's a self contained example:

from typing import Any, Callable, Protocol, TypeVar

T = TypeVar('T')

class ModelAfterValidator(Protocol):
    @staticmethod
    def __call__(
        self: T,
    ) -> T:
        ...

def model_validator() -> Callable[[ModelAfterValidator], Any]:
    raise NotImplementedError


class UserModel:
    @model_validator()
    def check_passwords_match(self) -> 'UserModel':
        raise NotImplementedError

I guess the whole @staticmethod thing on a Protocol with self: <type> is to blame. Open to better suggestions.

@adriangb
Copy link
Contributor

adriangb commented Jul 7, 2023

@Kludex can you update the title to something along the lines of "Protocol for an instance method taking a self argument"

@Kludex Kludex changed the title Union of Protocols failing (?) Protocol for an instance method taking a self argument Jul 8, 2023
@adriangb
Copy link
Contributor

I think we can just use Callable for now, but is there any way to require a self parameter as the first argument and that the return type be the same type (which is what I was attempting to do originally)?

@erictraut
Copy link

I'm not able to repro the original bug in this bug report using mypy 1.5. Perhaps it was fixed in a recent release, or perhaps the pydantic library has changed since the original bug report.

@adriangb
Copy link
Contributor

Pydantic has changed but the self contained example in #15620 (comment) should still be valid

@hauntsaninja
Copy link
Collaborator

I think mypy is correct to complain about #15620 (comment) ; pyright does as well. check_passwords_match does not have the right return type in the presence of UserModel subclasses. If you change the annotation to def check_passwords_match(self: T) -> T:, both mypy and pyright accept it

@adriangb
Copy link
Contributor

I guess then let me broaden the question: how can I type this decorator to enforce that it get applied to an instance method? In the real world model_validator takes a mode={before,after} parameter that uses overloading to satisfy two different signatures. The before case must be a class method and the after case must be an instance method. The classmethod isn't hard to enforce, it's instance methods that I'm struggling with. Making the first argument be self seemed like a reasonable compromise.

@hauntsaninja
Copy link
Collaborator

I'm not sure exactly I understand, but in pydantic case I think you can try a self: BaseModel annotation

@hauntsaninja hauntsaninja closed this as not planned Won't fix, can't repro, duplicate, stale Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants