Author mark.dickinson
Recipients Alexander.Belopolsky, amaury.forgeotdarc, eric.araujo, fredrikj, jess.austin, jribbens, mark.dickinson, r.david.murray, tleeuwenburg@gmail.com, vstinner, webograph
Date 2010-04-19.20:36:58
SpamBayes Score 4.05095e-10
Marked as misclassified No
Message-id <1271709421.29.0.786522510434.issue2706@psf.upfronthosting.co.za>
In-reply-to
Content
By the way, the patch looks good to me, as far as it goes, and I'm +1 on adding all this.  I only have the tiniest of nits:

 - the patch deletes a line at the top of Lib/test/test_datetime.py, for no apparent reason

 - in delta_add, I think the added 'if' block should be an 'else if' (yes, I know this is excessively picky ;)

 - the brace style for if blocks is inconsistent:  please put the opening brace on the same line as the if.

One other thought:  with this division operation added, it might be nice to add constants like td_hour, td_minute, etc. to the module.  Then the perennial 'how can I convert my timedelta x to minutes' questions could be answered with 'just do x/td_minute'.  I would personally find x/td_second to be a more obvious and natural way to find the total number of seconds in a timedelta than x.total_seconds.  I also quite like the idea of being able to create a 2.5-hour timedelta with something like

2*td_hour + 30*td_minute

On the other hand, maybe such constants would just be added clutter, since it's easy to create them when needed.
History
Date User Action Args
2010-04-19 20:37:01mark.dickinsonsetrecipients: + mark.dickinson, jribbens, amaury.forgeotdarc, vstinner, jess.austin, fredrikj, webograph, eric.araujo, r.david.murray, tleeuwenburg@gmail.com, Alexander.Belopolsky
2010-04-19 20:37:01mark.dickinsonsetmessageid: <1271709421.29.0.786522510434.issue2706@psf.upfronthosting.co.za>
2010-04-19 20:36:59mark.dickinsonlinkissue2706 messages
2010-04-19 20:36:58mark.dickinsoncreate