Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inconsistent arithmetic in subtraction of hms() / periods #503

Closed
anderstorring opened this issue Dec 29, 2016 · 1 comment
Closed

inconsistent arithmetic in subtraction of hms() / periods #503

anderstorring opened this issue Dec 29, 2016 · 1 comment

Comments

@anderstorring
Copy link

Hi!

When I try to subtract a period object created by hms() from another I find it gives faulty results. Searching to see if I had misused period subtraction I found this post on Stackoverflow by Dirk Eddelbuettel which seems to suggest that subtraction of periods is legit.

Example of expected correct result when the subtrahend is less than the minuend:

> hms("12:34:45") - hms("10:10:00")
[1] "2H 24M 45S"

Unclear result when subtracting and the minute or second subtrahend part is greater than the minuend:

> hms("20:03:00") - hms("19:55:00")
[1] "1H -52M 0S"
> hms("20:03:00") - hms("19:55:00") + hms("-01:52:00")
[1] "0S"

while "correct" it would be much nicer if the result of the subtraction as with addition is expressed as

[1] "8M 0S"

another example

> hms("12:34:45") - hms("11:50:00")
[1] "1H -16M 45S"

where you would expect
[1] "44M 45S"

Perhaps it would be easier to understand and use if there was a default automatic rollover from seconds -> minutes -> hours when doing additions and subtractions like when you construct a period using hms(...,roll=TRUE).

Here is another situation but somewhat contrived. Since lubridate can create hms/period objects with a greater amount of HH:MM:SS than the conventional 23:59:59 another subtraction inconsistency as compared to addition arises:

Addition is ok:

> hms("13:15:45") + hms("14:25:45")
[1] "27H 40M 90S"

and then the overflow seconds can be corrected by adding another hms() call, although somewhat inconveniently in comparison to the otherwise sleek syntax in lubridate:

hms(hms("13:15:45") + hms("14:25:45"),roll=TRUE)
"27H 41M 30S"

with subtraction:

> hms("13:15:15") - hms("14:25:95")
[1] "-1H -10M -80S"
> hms(hms("13:15:15") - hms("14:25:95"),roll=TRUE)
[1] NA
Warning message:
In .parse_hms(..., order = "HMS", quiet = quiet) :
  Some strings failed to parse

In any case, I would like to thank you so much for lubridate which has made working with dates and times so much easier!

@vspinu
Copy link
Member

vspinu commented Dec 30, 2016

The issue of rolling was discussed in #417.

You seem to be bothered mostly by negative components which are there to accommodate negative periods. In your last example hms("13:15:15") - hms("14:25:95") needs to be negative. So in order to "fix" your problem one would need to check if the period is negative in absolute sense and convert all components to negatives or positives accordingly. This might work for hms parts but not for large periods that include months and years. So, when you roll do you stop at days, weeks or go further to months and years? Add to this the extra computational burden on every operation and it simply becomes way too much trouble for little bang.

Note that your problem is primarily about presentation. All arithmetic operations will work just fine on those because they represent the same period.

As to the final NA and warning, it's a misuse of hms which is a parsing function. The outer call converts to character and then parses them back. It might be a bug in the parser though. I will check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants