Author pitrou
Recipients amaury.forgeotdarc, belopolsky, brett.cannon, brian.curtin, daniel.urban, lemburg, mark.dickinson, pitrou, r.david.murray, rhettinger, techtonik, tim.peters, vstinner
Date 2010-07-01.16:09:04
SpamBayes Score 0.000990357
Marked as misclassified No
Message-id <1278000546.87.0.940295534711.issue7989@psf.upfronthosting.co.za>
In-reply-to
Content
> In any case, my threshold for moving this forward is for someone to
> review the code in sandbox.

Ok some comments:

- I find the _cmp() and __cmp() indirection poor style in 3.x, especially when you simply end up comparing self._getstate() and other._getstate() (it is also suboptimal because it can do more comparisons than needed)

- Shouldn't __eq__ and friends return NotImplemented if the other type mismatches, to give the other class a chance to implement its own comparison method? that's what built-in types do, as least
(this would also make _cmperror useless)

- Using assert to check arguments is bad. Either there's a risk of bad input, and you should raise a proper error (for example ValueError), or there is none and the assert can be left out.

- Starting _DAYS_IN_MONTH with a None element and then iterating over _DAYS_IN_MONTH[1:] looks quirky

- Using double-underscored names such as __day is generally discouraged, simple-underscored names (e.g. _day) should be preferred

- Some comments about "CPython compatibility" should be removed

- Some other comments should be reconsidered or removed, such as "# XXX The following should only be used as keyword args" or "XXX Buggy in 2.2.2"

- Some things are inconsistent: date uses bytes for pickle support, time uses str for the same purpose
History
Date User Action Args
2010-07-01 16:09:07pitrousetrecipients: + pitrou, lemburg, tim.peters, brett.cannon, rhettinger, amaury.forgeotdarc, mark.dickinson, belopolsky, vstinner, techtonik, r.david.murray, brian.curtin, daniel.urban
2010-07-01 16:09:06pitrousetmessageid: <1278000546.87.0.940295534711.issue7989@psf.upfronthosting.co.za>
2010-07-01 16:09:05pitroulinkissue7989 messages
2010-07-01 16:09:04pitroucreate