Alexander Belopolsky <belopol...@users.sourceforge.net> added the comment:
Thanks a lot for the review. Please see my replies below. On Thu, Jul 1, 2010 at 12:09 PM, Antoine Pitrou <rep...@bugs.python.org> wrote: .. > - 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) > I agree. Do you think I should just define __lt__ and use functools.total_ordering decorator? Note that current implementation mimics what is done in C, but I think python should drive what is done in C and not the other way around. > - 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) This is a tricky part. See issue #5516. I would rather not touch it unless we want to revisit the whole comparison design. > > - 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. > I disagree. Asserts as executable documentation are good. I know, -O is disfavored in python, but still you can use it to disable asserts. Also I believe most of the asserts are the same in C version. > - Starting _DAYS_IN_MONTH with a None element and then iterating over > _DAYS_IN_MONTH[1:] looks quirky > Would you rather start with 0 and iterate over the whole list? It may be better to just define it as a literal list display. That's what C code does. > - Using double-underscored names such as __day is generally > discouraged, simple-underscored names (e.g. _day) should be preferred > I think in this case double-underscored names are justified. Pickle/cPickle experience shows that people tend to abuse the freedom that python implementations give to subclasses and then complain that C version does not work for them. I think __ name mangling will be a better deterrent than _ is private convention. > - Some comments about "CPython compatibility" should be removed > Why? The goal is to keep datetime.py in sync with datetimemodule.c, not to replace the C implementation. C implementation will still be definitive. > - Some other comments should be reconsidered or removed, such as > "# XXX The following should only be used as keyword args" This one I was actually thinking about making mandatory by changing the signature to use keyword only arguments. I am not sure if that is well supported by C API, though. > or "XXX Buggy in 2.2.2" Yes, a review of XXXs is in order. > > - Some things are inconsistent: date uses bytes for pickle support, > time uses str for the same purpose Already fixed. ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue7989> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
