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

Support __eq__ for pyclass #2089

Closed
Lunarmagpie opened this issue Jan 6, 2022 · 8 comments · Fixed by #3203
Closed

Support __eq__ for pyclass #2089

Lunarmagpie opened this issue Jan 6, 2022 · 8 comments · Fixed by #3203

Comments

@Lunarmagpie
Copy link

I don't think is possible to create a class like this in pyo3. I couldn't find a way to write an equality method in the docs.

class SomeClass:
    def __eq__(self, other):
        return True

I need to be able to write an implementation for this so I can make a python wrapper around rust enums. I found that there wasn't a way to do an equality check. I'm considered writing an is_equal_to method then making another class in python that knows how to use is_equal_to when you call __eq__. I want to avoid this because I think its clunky and requires a lot of boilerplate. Forcing an end user to use a method like is_equal_to is jarring so I can't do that.

@davidhewitt
Copy link
Member

At the moment you should implement __richcmp__, which is the C api where all these operators are defined. See #1625 (comment) for a current example, unfortunately the docs aren't great for it right at this moment.

Let's leave this issue open, because I think with the new #[pymethods] magic methods there might be a way to enable users to write the usual Python operations like __eq__. I just need to find a chance to have a play with enabling this.

@Lunarmagpie
Copy link
Author

Thanks for explaining!

@ssokolow
Copy link

To me, it feels like the most ergonomic thing would be to offer something like #[derive(Eq, Hash, PartialEq, PyEq, PyHash)].

@davidhewitt
Copy link
Member

@ssokolow there's been suggestions previously to have #[pyclass(dataclass)] to do exactly that, see #1375 (comment)

Just not got around to implementing!

@ssokolow
Copy link

ssokolow commented May 19, 2022

I don't necessarily want dataclass support... just the ability to proxy implementations through easily.

(My most recent example would have been fine with #[pyclass(dataclass)] since it's just porting an IntEnum to Rust as part of migrating as much as possible of a PyQt codebase into its Rust backend, but I've had cases where I would want to easily expose derives as the corresponding dunder methods without having to make all fields public or refactor the thing to put the private methods in a field containing an opaque object.)

(Even then, since I was also porting from rust-cpython to PyO3 at the same time and this is my first use of PyO3, I wound up turning Roles.guess(name) into a guess_role(name: &str) -> Role free function because I'm probably going to refactor things enough in future passes anyway that I didn't feel like front-loading my understanding of how class methods work in PyO3.)

@davidhewitt
Copy link
Member

Ah, interesting. I will have a think about whether there's a nice way to do this - it might solve some papercuts I've had too!

@alex
Copy link
Contributor

alex commented Apr 10, 2023

Being able to define __eq__, instead of __richcmp__, would be a big QoL improvement. The specific advantages are: Not needing to open-code a not-equal or order-comparison implementations, since those could just offer standard behaviors.

@davidhewitt
Copy link
Member

Finally pushed #3203 to implement this!

bors bot added a commit that referenced this issue Jun 4, 2023
3203: support ordering magic methods for `#[pyclass]` r=adamreichold a=davidhewitt

Closes #2089 

This adds `__lt__`, `__le__`, `__eq__`, `__ne__`, `__gt__`, and `__ge__` as per the Python implementations of what we call `__richcmp__`.

There's a UI test confirming that the user cannot implement split forms and `__richcmp__` simultaneously.

There's also a benchmark comparing implementing these split methods against using `__richcmp__`. I couldn't see a meaningful performance difference, so I'm tempted to deprecate `__richcmp__`, given that's not a magic method which exists in Python. Potentially we can provide options such as the opt-in `#[pyclass(eq, ord)]` to avoid boilerplate for people who don't want to implement six different methods.



Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
@bors bors bot closed this as completed in dbf7b23 Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants