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

IsInstance does not work for Pydantic BaseModels #61

Open
StefanBRas opened this issue Apr 20, 2023 · 3 comments
Open

IsInstance does not work for Pydantic BaseModels #61

StefanBRas opened this issue Apr 20, 2023 · 3 comments

Comments

@StefanBRas
Copy link

It seems like IsInstance will never be equal to a pydantic model:

from dirty_equals import IsInstance
from pydantic import BaseModel

class A(BaseModel): pass

print(A() == IsInstance(A)) # False
print(A() == IsInstance[A]) # False

class B: pass

print(B() == IsInstance(B)) # True
print(B() == IsInstance[B]) # True

Interestingly enough it works perfectly with IsPartialDict:

from dirty_equals import IsPartialDict, IsNegativeInt
from pydantic import BaseModel

class C(BaseModel):
    a: int
    b: str

print(C(a=-1,b="a") ==  IsPartialDict(a=IsNegativeInt)) # True
print(C(a=-1,b="a") ==  IsPartialDict(a=IsNegativeInt, b=1)) # False

Maybe it's also nice to document that IsPartialDict works with pydantic models without having to call model_instance.dict()?

Tested with dirty-equals version 0.50 and pydantic versions 1.9.0 and 1.10.7.

@StefanBRas
Copy link
Author

Ah, I see now that pydantic does

def __eq__(self, other: Any) -> bool:
    if isinstance(other, BaseModel):
        return self.dict() == other.dict()
    else:
        return self.dict() == other

So it will do the dict equality check instead.

So in the other order, it works:

print(IsInstance(A) == A()) # True
print(IsInstance[A] == A()) # True

I guess it can't really be changed, unless you were like the maintainer of pydantic or something (😉) and I'd doubt they would ever do that because It would break a lot of peoples code.

Feel free to close the issue, I'll leave it up so you can decide if there should be something in the documentation about this.

@samuelcolvin
Copy link
Owner

That's changing in V2, also the __eq__ logic shouldn't affect the IsInstance() check.

@alexmojaki
Copy link
Contributor

That's changing in V2

Indeed, A() == IsInstance(A) is True with pydantic V2.

also the __eq__ logic shouldn't affect the IsInstance() check.

Not sure what you mean here, but it makes sense that the old BaseModel.__eq__ prevented IsInstance from working since it doesn't go through the usual NotImplemented pattern. It evaluates self.dict() == other which leads to IsInstance checking if a dict is an instance of a model class. So I think it's impossible to fix this for pydantic v1 without changes to pydantic v1 or some monkeypatching here.

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

No branches or pull requests

3 participants