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 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.19:13:47
SpamBayes Score 1.1574178e-07
Marked as misclassified No
Message-id <1278011624.3211.24.camel@localhost.localdomain>
In-reply-to <1278002560.76.0.579280516538.issue7989@psf.upfronthosting.co.za>
Content
> 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.
History
Date User Action Args
2010-07-01 19:13:49pitrousetrecipients: + 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:48pitroulinkissue7989 messages
2010-07-01 19:13:47pitroucreate