[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Roundup Robot added the comment: New changeset 1cc413874631 by R David Murray in branch 'default': whatsnew: total_ordering supports NotImplemented (#10042) http://hg.python.org/cpython/rev/1cc413874631 -- ___ Python tracker

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Raymond Hettinger added the comment: Thanks Nick and Katie. This looks great. :-) -- ___ Python tracker ___ ___ Python-bugs-list mail

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Roundup Robot added the comment: New changeset ad9f207645ab by Nick Coghlan in branch 'default': Close #10042: functools.total_ordering now handles NotImplemented http://hg.python.org/cpython/rev/ad9f207645ab -- resolution: -> fixed stage: commit review -> committed/rejected status: ope

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: The committed patched was based directly on Katie's last version, without my enum changes. Raymond - feel free to tweak the wording on the docs notes or the explanatory comment if you see anything that could be improved. --

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Raymond Hettinger added the comment: One other thought: The OrderedEnum example should not use the total ordering decorator. To the extent that the docs are trying to teach how to use Enum, they should focus on that task and not make a side-trip into the world of class decorators. And to

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Raymond Hettinger added the comment: Nick, the latest version of the patch looks like a correct solution. Before applying, please add a block comment showing why these shenanigans are necessary (i.e. the use self.__lt__ instead of the less-than operator because the former doesn't fall into rec

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: Raymond, I'm happy to leave this until alpha 4, but I'm raising the priority a bit since I think the inclusion of Enum in the standard library increases the chances of people wanting to use functools.total_ordering to avoid writing out the comparison methods in

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: Updated patch that includes the simplified OrderedEnum example in the enum docs and also updates the enum tests to check that type errors are correctly raised between different kinds of ordered enum. -- nosy: +ethan.furman priority: low -> normal __

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: Raymond, do you still want to look at this one? Otherwise I'll finish it up and commit it before the next alpha (I'll check the example in the enum docs to see if it can be simplified, too). -- ___ Python tracker

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Drekin added the comment: Hello, I have run into this when I wanted to use OrderedEnum and the example in enum docs seemed too repetitive to me. It's nice to know that it's being worked on. -- nosy: +Drekin ___ Python tracker

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: Thanks Katie - Raymond, the patch is ready for review now If you're happy with it, then the only other things it should need prior to commit are NEWS and ACKS entries (I think it's too esoteric a fix to mention in What's New). -- stage: needs patch -> c

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Katie Miller added the comment: Nick is correct; a logic bug was introduced during refactoring, which is fixed in the attached patch. The tests introduced with my original patch cover cases where an operation is not implemented, so it would be inappropriate to add a test case there that would

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: Agreed. I had actually assumed this would be 3.4 only, otherwise I wouldn't have suggested using the new subtest feature in the test case. -- ___ Python tracker

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Raymond Hettinger added the comment: After more thought, I'm changing this to Py3.4 only. For prior versions, I'm content to document that there is no support for NotImplemented, and if that is needed, then people should write-out all six rich comparisons without using the total ordering deco

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Jim Jewett added the comment: > Since this is such an incredibly niche edge case > (the ordered comparison has to return NotImplemented > while __eq__ returns True), *and* the types are explicitly supposed to ordered, based on what is being tested > I remaining consistent with the existing b

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: I think I spotted a logic bug in _not_op_and_not_eq (it uses "or" instead of "and" in the final line) , so I suspect we still have a missing test case in the latest patch. (My fault - I should have suggested using coverage.py to ensure all the branches were covere

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Raymond Hettinger added the comment: Nick, let me know when you think it is ready and I'll review the patch. -- ___ Python tracker ___ ___

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Katie Miller added the comment: Attached is a new patch, which includes Nick's logic from msg140493, some of the code readability changes Jim suggested in msg151989 (but not the behavioural changes), and associated tests. On Nick's advice, I have also replaced the dunder equals calls with the

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: OK, I had misunderstood the way Jim's code works (it still coerces a "False" result for __eq__ into NotImplemented if the ordered comparison returns NotImplemented). However, I spent some more time tinkering with it today (see https://bitbucket.org/ncoghlan/misc

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: I'm actually not sure which of us is correct - Katie and I will be looking into it further today to compare the existing implementation, my proposal and yours to see if there's a clear winner in terms of consistent. It may be that we end up choosing the version

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Jim Jewett added the comment: On Mon, Jul 8, 2013 at 3:30 AM, Nick Coghlan wrote: > The key difference between them relates to the following different approaches > to handling unknown types in __eq__: > @functools.total_ordering > class TotallyOrderedEqualsReturnsFalse: ... >def __eq__(sel

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: As part of this, I finally reviewed Jim's proposed alternate implementations for the helper functions. Katie's patch used my version while I figured out the differences in behaviour :) The key difference between them relates to the following different approaches

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Katie Miller added the comment: Attaching patch with Nick Coghlan's suggested code from msg140493 and associated tests. The tests extend the single test case that had already been added for earlier changes based on this bug. The tests check that a TypeError is raised, rather than a stack overf

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Nick Coghlan added the comment: Raymond, one of the devs here at the PyCon AU sprints has been looking into providing an updated patch for this. Do you mind if I reassign the issue to myself to review their patch (once it is uploaded)? -- ___ Python

[issue10042] functools.total_ordering fails to handle NotImplemented correctly

Changes by Nick Coghlan : -- title: total_ordering -> functools.total_ordering fails to handle NotImplemented correctly ___ Python tracker ___ __