gweis / isodate

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

Duration.total_seconds() fails for months and years #44

Open pilgrimbeart opened 7 years ago

pilgrimbeart commented 7 years ago
>>> import isodate
>>> isodate.parse_duration("P3M").total_seconds()
0.0
reinhrst commented 6 years ago

Of course it fails.... How many seconds are there in a month? Depends on whether it's February or August, and this is not known for a P3M. However I would argue that it should return an exception, rather than 0.0..

mristin commented 6 years ago

@reinhrst , if you look at the documentation of datetime.timedelta from https://docs.python.org/3/library/datetime.html:

timedelta.total_seconds()

Return the total number of seconds contained in the duration. Equivalent to td / timedelta(seconds=1).

Note that for very large time intervals (greater than 270 years on most platforms) this method will lose microsecond accuracy. New in version 3.2.

I would also expect the total seconds to return the duration in seconds, not the seconds part of the duration.

theferrit32 commented 6 years ago

Timedelta has a total_seconds but it does not handle any sort of conversion from months to seconds. Month is not a standardized time interval unit. It is only useful for scheduling tasks that run on monthly or yearly intervals, to allow more human-understandable schedules (execute this every 5th day of every month, etc). The number of seconds between each execution will not be the same though, so there is no way to map it to a number of seconds, or even days, unless you provide which two months you're talking about, and which year it is (since month lengths vary by year). That seems like it is not useful. For that same reason month/year is not a unit in the timedelta module, week is the largest standardized duration unit.

I agree with @reinhrst that this should probably throw an exception instead of returning 0, because it is an invalid operation.

mmerickel commented 1 year ago

It would be nice to see this throw an exception to avoid any misusage in the API.