Message109069
> I agree. Do you think I should just define __lt__ and use
> functools.total_ordering decorator?
I had forgotten about functools.total_ordering. Yes, very good idea.
> 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.
I think the Python version doesn't have to mimic every exact quirk of
the C version. I think it's good if the code is idiomatic Python.
> I disagree. Asserts as executable documentation are good.
I am talking specifically about this kind of assert:
assert 1 <= month <= 12, 'month must be in 1..12'
I think it should be replaced with:
if month < 1 or month > 12:
raise ValueError('month must be in 1..12')
I don't think it's reasonable to disable argument checking when -O is
given. Furthermore, AssertionError is the wrong exception type for this.
On the other hand, I do agree that most asserts in e.g.
timedelta.__new__ are good.
> > - 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.
Hmm, I wrote that comment before discovering that it is useful for
actual data to start at index 1. You can forget this, sorry.
> 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.
Ah, but the Python datetime implementation will be automatically
shadowed by the C one; you won't end up using it by mistake, so people
should not ever rely on any of its implementation details.
To give a point of reference, the threading module used the __attribute
naming style for private attributes in 2.x, but it was converted to use
the _attribute style in 3.x.
(one genuine use for it, by the way, is to make it easy to test
implementation-specific internal invariants in the test suite)
> > - 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.
Yes, but talking about CPython compatibility in the CPython source tree
looks puzzling. You could reword these statements, e.g. "compatibility
with the C implementation".
> > - 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.
That would be an API change and would break compatibility. Are you sure
you want to do it?
> I am not sure if that is well supported by C API, though.
Not at all. You would have to analyze contents of the keywords dict
manually. |
|
Date |
User |
Action |
Args |
2010-07-01 19:13:49 | pitrou | set | recipients:
+ pitrou, lemburg, tim.peters, brett.cannon, rhettinger, amaury.forgeotdarc, mark.dickinson, belopolsky, vstinner, techtonik, r.david.murray, brian.curtin, daniel.urban |
2010-07-01 19:13:48 | pitrou | link | issue7989 messages |
2010-07-01 19:13:47 | pitrou | create | |
|