Martin Panter added the comment: A Mercurial export patch should work with the Reitveld review thing if it is a single revision (and based on a public revision). Or you could try getting a plain diff from the base revision. Or for changes that are independent, separate patches based off a common public revision. Please at least fold your fixup commits into the original commits; that would make it a lot easier to review.
You have a stray print() call, probably in Hashable.__subclasshook__(). For a the do-nothing __reversed__() implementation, do you think “yield from ()” would be clearer than “while False”? Or even “return iter(())”? What’s the advantage of having Reversed inherit Iterable? How does the subclass hook interact with classes that implement __reversed__() but not __iter__()? I don’t see how the self.assertFalse(issubclass(K, Reversible)) test could pass. Should the new Reversed class be excluded from collections.__all__, or is it not worth it? I find the lambda generator a bit quirky in test_Reversible(). Maybe it would be better as an ordinary non-lambda generator with a yield statement. It would be good to have a versionadded notice for Reversible. I think there should also be one for allowing None for all special methods. Instead of self.assertEqual(list(reversed(R())), list(reversed([]))), why not check it is equal to an empty list directly? In test_contains.py, I would either write lambda: 0 in bc, or use the “with self.assertRaises()” form. Finally, if setting a binary operator method to None means that operation is not available, I find it a bit surprising that doing this prevent the other operand’s method from being tried. I guess you don’t want to change the implementation, but perhaps the documentation of the binary operator methods could be clarified. ---------- stage: -> patch review type: -> enhancement _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue25958> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
