Skip to content

[WIP] Fix comparison for classes implementing __eq__.#51

Open
icemac wants to merge 6 commits intomasterfrom
issue-50
Open

[WIP] Fix comparison for classes implementing __eq__.#51
icemac wants to merge 6 commits intomasterfrom
issue-50

Conversation

@icemac
Copy link
Copy Markdown
Member

@icemac icemac commented Dec 30, 2017

There are currently only two new failing tests:

  • Comparing an object with a proxy of the object fails (but comparing the proxy with the object is fine!)
  • Comparing a proxy with itself fails

But these tests run fine when deleting the __eq__ method on the Something class which is the class of self.x.

Fixes #50.

@mgedmin
Copy link
Copy Markdown
Member

mgedmin commented Dec 30, 2017

This is a WIP, right? I'm a bit confused by the commit message that says "Fix this and that" but only adds two failing unit tests.

@icemac
Copy link
Copy Markdown
Member Author

icemac commented Dec 30, 2017

@mgedmin When complete this PR will fix #50. I already added this to the description. Currently it is WIP hoping for suggestions resp. help to fix the test failures.

@icemac icemac changed the title Fix comparison for classes implementing __eq__. [WIP] Fix comparison for classes implementing __eq__. Aug 18, 2018
Michael Howitz added 3 commits April 2, 2019 21:24
Update to current Python versions.
5ae2445 
Additionally we have to pin an older `Sphinx` version to keep supporting
Python 2.
Fix comparison for classes implementing __eq__.
4fc05a1 
There are currently only two new failing tests:

* Comparing an object with a proxy of the object fails (but comparing the proxy with the object is fine!)
* Comparing a proxy with itself fails

But these tests run fine when deleting the `__eq__` method on the `Something` class which is the class of `self.x`.
Unwrap other if it is a proxy, so the proxy can compare equal to itself.
7684a5f
@icemac icemac changed the base branch from master to update April 2, 2019 19:27
@icemac icemac changed the base branch from update to master April 6, 2019 05:51
@icemac icemac self-assigned this Apr 6, 2019
@icemac
Copy link
Copy Markdown
Member Author

icemac commented Apr 6, 2019

Hm, this PR does not replace the fix in zopefoundation/zope.keyreference#6. I have to investigate further.

@icemac
Copy link
Copy Markdown
Member Author

icemac commented Apr 21, 2020 •
edited
Loading

These changes do not help for zopefoundation/zope.keyreference#6 because they happen on the wrong end: If self is security proxied it now gets unwrapped. But we need to unwrap other as that object is security proxied not self.

@icemac
Copy link
Copy Markdown
Member Author

icemac commented Apr 21, 2020

These changes do not help for zopefoundation/zope.keyreference#6 because they happen on the wrong end: If self is security proxied it now gets unwrapped. But we need to unwrap other as that object is security proxied not self.

That were my internal notes but looking into the diff of this PR tells a completely different story.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@icemac icemac

Labels

do not merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Objects having __eq__ behave different from ones which only have __cmp__

2 participants

@icemac @mgedmin