Vinay Sajip <vinay_sa...@yahoo.co.uk> added the comment: > Antoine Pitrou <pit...@free.fr> added the comment:
> It's not proscribed, but trying to remove the "self." because it's > supposed to be more readable is a bit of a strange thing to do. > Also, people reading the test suite should be accustomed to > "self.assertEqual" anyway, so there's no point trying to hide it. It wasn't particularly about self - I'm not against it. Anyway, it's not a big deal for me, so I've added the selves back :-) > Error checking can't just be probabilistic. Perhaps there's a bug in the > file-like object; or perhaps it is a non-blocking IO object and read() > will return None at times. You're right, so I've raised a TypeError if PyBytes_Check fails in r_string. > Well, it wouldn't fail any slower if you didn't do it, since you need to > call read() very soon anyway (presumably as part of the same call to > marshal.load()). Failing "fast" doesn't seem to bring anything here. My > vote is for removing the complication. Actually I misremembered the complete reason for the call - it was there to additionally check that the passed object has a read method. I also realised - duh - that I can read zero bytes and still get an empty bytes object back, so I've done that, and it does look cleaner. I've also reorganised the marshal_load function a little so it flows better. Your other points have also all been addressed: I've seen that the original test with the long binary data was exercising the same functionality as Engelbert Gruber's later patch was testing. So I've removed my earlier test entirely and added tuples and lists into the data being marshalled in Engelbert's version of the test. As a result of these changes, the zlib/base64 dependency goes away. The new "readable" field of the C struct has a comment saying what it is. The commented out r_byte macro has been removed. Thanks, again, for the review! ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue12291> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
