gweis / isodate

ISO 8601 date/time parser
BSD 3-Clause "New" or "Revised" License
148 stars 58 forks source link

added astimedelta() to Duration #49

Closed mristin closed 6 years ago

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.03%) to 94.466% when pulling 9dd26b36a0ead88d0c278d861bcc49eeb9903a41 on mristin:added-astimedelta-to-Duration into ce635a7a483effb3fc246721cfb5a8a7b5174ab5 on gweis:master.

mristin commented 6 years ago

Hi! Any chance you might consider merging this change soon (in the next 2-3 weeks)? I would be very thankful for a response, so that we can either wait for this feature in your package or implement it in our code base independently.

gweis commented 6 years ago

Hi,

To correctly transform a Duration to a timedelta you need a reference date. Otherwise you are just guessing, that a year has 365 days, a month has 30 days, which is not always the case.

There was a discussion around this in #42. (There are a couple of other suggestions in that thread that should probably make it into the library). However, I believe the cleanest way would be to use a reference date to convert to a timedelta in a generic way. Otherwise if you make assumptions about the length of a year or a month, you'd be probably better off to be explicit in your code, rather than hiding it in a library.

mristin commented 6 years ago

Hi @gweis In most cases where we use durations they are stored in the config files and indicate the life time of certain objects. I assume this is not such a rare use case and could find general application. So for us the average month and year are fine.

What about having totimedelta() with both start and end None return the time delta with the average month and year?

mristin commented 6 years ago

Hi @gweis , Have you had a chance to consider the arguments to totimedelta()? Or what about adding the function to_timedelta_avg()? Please let me know what you prefer, and I'll gladly make a new pull request.

gweis commented 6 years ago

Hi sorry for the late reply,

I have never seen a use case where I had a precise duration and only needed an estimate to what date that would lead me.

Wouldn't it be the same if you supply a start date of datetime.now() or whatever random start date you want? My expectation would be that if something has a life time of a month, it would be from 1st to 1st. Otherwise the life time should be represented as days in the first place.

mristin commented 6 years ago

Hi @gweis , Please also apologize for the late reply from my side -- I missed your comment in my email log and assumed wrongly there was no activity on this pull request.

I have never seen a use case where I had a precise duration and only needed an estimate to what date that would lead me.

Think of the use case where the duration models a sort of age (e.g., how long to keep some cached data, or how long to keep the private data in the database etc.). You would always use the same duration even though the reference point would constantly change. While I agree that it is perfectly possible to store the duration in days, writing P180D is way less practical than P3M. If it's five years then the period becomes quite unreadable: P1825D. The month in this context obviously does not refer to a calendar month.

We can also close the issue if you feel strongly about this use case and think that it would add clutter to the library.

gweis commented 6 years ago

You would always use the same duration even though the reference point would constantly change

That's exactly my point. What data retention do you promise here? P3M can be seen as something from 90 to 92 days. If you say a month is 30 days, then you actually promise 90 days.

Same with years. Do you promise 365 days or 366 days when you say P1Y ?

totimedelta(datetime.now()), will always give you a timedelta that's correct with the given reference date.

Or think of birthdays in terms of age. Someone has lived P10Y years, and you add one year. By just estimating how long this period is, you'll probably send out birthday greeting cards on the wrong day. (not by much but still wrong).

Fell free to re-open in case if you think I am getting something really, wrong here.