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

__ne__ does not defer to custom __eq__ #585

Closed
Solumin opened this issue Nov 17, 2023 · 2 comments · Fixed by #593
Closed

__ne__ does not defer to custom __eq__ #585

Solumin opened this issue Nov 17, 2023 · 2 comments · Fixed by #593

Comments

@Solumin
Copy link

Solumin commented Nov 17, 2023

Description

I've encountered unexpected behavior with custom-defined __eq__ methods:

class A(msgspec.Struct, eq=False):
  x: int
  y: int
  def __eq__(self, other):
    return self.__class__ == other.__class__ and self.x == other.x

x = A(1, 2)
y = A(1, 3)
print(x == y)  # True
print(x != y)  # True  <-- unexpected!

The default implementation of __ne__ just defers to __eq__, so x != y should be False. It seems that Struct has its own __ne__ implementation that does not do this.

Admittedly, this is a really weird case that would probably be solved by making y a private field (#199). I'd also be happy with just updating the docs to state that if you provide a custom __eq__ that you should also define __ne__.

@jcrist
Copy link
Owner

jcrist commented Nov 25, 2023

Thanks for the excellent reproducible issue, this has been fixed in #593.

Admittedly, this is a really weird case that would probably be solved by making y a private field (#199).

As part of that work I also intend to support field level configuration for eq/repr/init, matching the behavior of dataclasses and attrs. When that's done, you should be able to do something like this to get what you want:

class A(msgspec.Struct):
  x: int
  y: int = msgspec.field(eq=False)

@Solumin
Copy link
Author

Solumin commented Nov 25, 2023

Thank you for fixing this so quickly!

As part of that work I also intend to support field level configuration for eq/repr/init, matching the behavior of dataclasses and attrs.

Excellent, I think that will be a very helpful feature.

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 a pull request may close this issue.

2 participants