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

Fix __ge__ and __gt__ comparison methods #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mcb30
Copy link

@mcb30 mcb30 commented May 27, 2019

Quoting the Python documentation for sets:

"The subset and equality comparisons do not generalize to a total
ordering function. For example, any two nonempty disjoint sets are
not equal and are not subsets of each other, so all of the
following return False: a<b, a==b, or a>b."

It is therefore not possible to define ge as the inverse of lt
(and similarly gt as the inverse of le), since this will give
false positive results.

Fix by implementing ge and gt using equivalent logic to that
used in le and lt.

Signed-off-by: Michael Brown mbrown@fensystems.co.uk

Quoting the Python documentation for sets:

  "The subset and equality comparisons do not generalize to a total
   ordering function. For example, any two nonempty disjoint sets are
   not equal and are not subsets of each other, so all of the
   following return False: a<b, a==b, or a>b."

It is therefore not possible to define __ge__ as the inverse of __lt__
(and similarly __gt__ as the inverse of __le__), since this will give
false positive results.

Fix by implementing __ge__ and __gt__ using equivalent logic to that
used in __le__ and __lt__.

Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
@simonpercivall simonpercivall self-assigned this May 27, 2019
@mcb30
Copy link
Author

mcb30 commented Jun 2, 2019

Do you have any queries or requested changes? Would it help if I opened an issue to report the original bug that is fixed by this patch?

Thanks,

Michael

@azmeuk
Copy link

azmeuk commented Mar 18, 2020

@simonpercivall Any thoughts on this PR?

@idanmiara
Copy link

Until this issue is resolved here, may want to check out #38
As this issue is also fixed in https://pypi.org/project/stableset/

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 this pull request may close these issues.

4 participants