Comment 6 for bug 969928

Revision history for this message
Tomi Hukkalainen (tpievila) wrote :

> I think you want to raise NotImplementedError; not NotImplemented.

Well, as per the latter post, it doesn't change anything from simply
not defining the magic functions.

> I agree that there is some ambiguity between static times (days,
> hours, etc) and dynamic times (months, years). This already exists
> in __eq__ where 28 days is not equal to 1 month. It's also unclear
> what it means to add 1 month and 1 day to a datetime because it's
> not commutative.

It is perhaps not intuitively clear, but it is defined. See
http://labix.org/python-dateutil#head-72c4689ec5608067d118b9143cef6bdffb6dad4e.
Years are added first, then months, then days.

> One solution would be to assert that within a relativedelta static
> and dynamic times cannot be mixed within the same relativedelta.

This would be a severe limitation, I think.

> This would solve the ambiguity that exists between adding
> relativedelta(months=1, days=1) to datetime.datetime(2012, 2, 28): a
> month and a day could be added as distinct relativedelta objects to
> a datetime and it forces the user to explicitly set the ordering.

See above, the algorithm is not ambiguous. I agree that it's not
immediately clear, but it's also pretty obvious that there's no one
true way to calculate relativedeltas, so users probably (hopefully)
will check the exact behaviour from the docs, if it matters.

I think it's better to let users choose if they want to put the months
and days as separate objects.

> Going by this scheme, you could then raise NotImplementedError when
> comparing static and dynamic times, but still yield a sensible
> answer when comparing relativedeltas which contain static times.

Raising the error wouldn't change anything, Python2 would still
happily compare the two objects, with an undefined behaviour.

> This could be a breaking change for some people. But I think it's an
> improvement because it doesn't seem to be reasonable to rely on
> relativedelta(months=1, days=1). If you think it's an good way to
> proceed let me know and I'll whip up a patch. I've implemented it in
> my forked version already.

Nothing forces the users to rely on it. I do see the point about
comparisons being wonky, but this wouldn't actually help with that.