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 pitrou
Recipients Arfrever, amaury.forgeotdarc, benjamin.peterson, djc, georg.brandl, grubert, pitrou, vinay.sajip
Date 2011-06-30.19:26:09
SpamBayes Score 1.110223e-16
Marked as misclassified No
Message-id <1309461928.3214.15.camel@localhost.localdomain>
In-reply-to <1309461169.39295.YahooMailRC@web25805.mail.ukl.yahoo.com>
Content
> > - "assertEqual = self.assertEqual" is more of a  nuisance than anything else; 
> >you're making the code more complicated to read  just to save a few keystrokes; 
> >same for "assertIsInstance =  self.assertIsInstance"
> 
> I'm not sure how it's more complicated or harder to understand. I didn't do it 
> just to save keystrokes when writing, it's also to avoid noise when reading. IMO 
> this is a question of personal taste, or is this approach proscribed somewhere? 

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.

> > - in r_string(), what happens if  PyBytes_Check(data) is false? there should 
> >probably be a TypeError of some  sort
> 
> There is a PyBytes_Check in marshal_load which raises a TypeError if it fails. 
> The PyBytes_Check in r_string is only called in the path from marshal_load 
> (because p->readable is only non-NULL in that path) - so for the failure to 
> occur one would need an initial read to deliver bytes, and a subsequent read to 
> deliver non-bytes. Still, I have no problem adding a TypeError raising in 
> r_string if PyBytes_Check fails.

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.

> > - why do you call read(1) at  the beginning? it seems to make the code more 
> >complicated for no useful  purpose
> 
> I gave the reasoning in the comment just above the read(1). I agree it makes it 
> a little more complicated, but not onerously so - and the approach fails fast as 
> well as allowing operation with any stream, not just random-seekable ones.

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.
History
Date User Action Args
2011-06-30 19:26:10pitrousetrecipients: + pitrou, georg.brandl, vinay.sajip, amaury.forgeotdarc, grubert, benjamin.peterson, djc, Arfrever
2011-06-30 19:26:09pitroulinkissue12291 messages
2011-06-30 19:26:09pitroucreate