This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author martin.panter
Recipients abarnert, gvanrossum, martin.panter, ncoghlan, r.david.murray, serhiy.storchaka
Date 2016-01-05.06:03:37
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1451973817.88.0.0942148753787.issue25958@psf.upfronthosting.co.za>
In-reply-to
Content
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.
History
Date User Action Args
2016-01-05 06:03:37martin.pantersetrecipients: + martin.panter, gvanrossum, ncoghlan, r.david.murray, serhiy.storchaka, abarnert
2016-01-05 06:03:37martin.pantersetmessageid: <1451973817.88.0.0942148753787.issue25958@psf.upfronthosting.co.za>
2016-01-05 06:03:37martin.panterlinkissue25958 messages
2016-01-05 06:03:37martin.pantercreate