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

duration() ignores decimal elements when parsing #519

Closed
jspncr opened this Issue Feb 15, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@jspncr

jspncr commented Feb 15, 2017

> duration("1.5 hours")  
[1] "3600s (~1 hours)"
@cderv

This comment has been minimized.

Show comment
Hide comment
@cderv

cderv Feb 15, 2017

Contributor

I think it is a duplicate and fixed by #465.
Have you tried last dev version?

Contributor

cderv commented Feb 15, 2017

I think it is a duplicate and fixed by #465.
Have you tried last dev version?

@jspncr

This comment has been minimized.

Show comment
Hide comment
@jspncr

jspncr Feb 16, 2017

Yes, you are correct. Sorry, I should have checked the dev version first.

jspncr commented Feb 16, 2017

Yes, you are correct. Sorry, I should have checked the dev version first.

@cderv

This comment has been minimized.

Show comment
Hide comment
@cderv

cderv Feb 16, 2017

Contributor

No problem. You will next time :)
Can you close the issue as it is ok and already corrected ?

Contributor

cderv commented Feb 16, 2017

No problem. You will next time :)
Can you close the issue as it is ok and already corrected ?

@jspncr jspncr closed this Feb 16, 2017

@jspncr

This comment has been minimized.

Show comment
Hide comment
@jspncr

jspncr Feb 16, 2017

Wait, i'm an idiot. I tested duration(1.5, "hours"), not duration("1.5 hours")

The issue remains.

> duration("1.5 hours")
[1] "3600s (~1 hours)"
> duration(1.5, "hours")
[1] "5400s (~1.5 hours)"

(lubridate_1.6.0.9009)

jspncr commented Feb 16, 2017

Wait, i'm an idiot. I tested duration(1.5, "hours"), not duration("1.5 hours")

The issue remains.

> duration("1.5 hours")
[1] "3600s (~1 hours)"
> duration(1.5, "hours")
[1] "5400s (~1.5 hours)"

(lubridate_1.6.0.9009)

@jspncr jspncr reopened this Feb 16, 2017

@cderv

This comment has been minimized.

Show comment
Hide comment
@cderv

cderv Feb 16, 2017

Contributor

OK I see. duration takes two arguments num et units default to "seconds". num can take numeric or character, but it seems that when character, it do not handle decimal duration.

library(lubridate)
#> 
#> Attachement du package : 'lubridate'
#> The following object is masked from 'package:base':
#> 
#>     date
duration("1.5 hours")
#> [1] "3600s (~1 hours)"
duration("1 hour 30 min")
#> [1] "5400s (~1.5 hours)"
duration(1.5,  "hours")
#> [1] "5400s (~1.5 hours)"

behind the scene when num is character, it calls lubridate:::parse_period that do not handle decimal;

lubridate:::parse_period("1.5 hours")
#> [1] "1H 0M 0S"

that is converted to duration and returns "3600s (~1 hours)".

So,

  • either parse_period is supposed to handle decimal in the character parsing and it should be corrected, tested and documented
  • or it is not intended and it should at least be documented.

Not my decision, we should wait for @vspinu advice.

Contributor

cderv commented Feb 16, 2017

OK I see. duration takes two arguments num et units default to "seconds". num can take numeric or character, but it seems that when character, it do not handle decimal duration.

library(lubridate)
#> 
#> Attachement du package : 'lubridate'
#> The following object is masked from 'package:base':
#> 
#>     date
duration("1.5 hours")
#> [1] "3600s (~1 hours)"
duration("1 hour 30 min")
#> [1] "5400s (~1.5 hours)"
duration(1.5,  "hours")
#> [1] "5400s (~1.5 hours)"

behind the scene when num is character, it calls lubridate:::parse_period that do not handle decimal;

lubridate:::parse_period("1.5 hours")
#> [1] "1H 0M 0S"

that is converted to duration and returns "3600s (~1 hours)".

So,

  • either parse_period is supposed to handle decimal in the character parsing and it should be corrected, tested and documented
  • or it is not intended and it should at least be documented.

Not my decision, we should wait for @vspinu advice.

@vspinu

This comment has been minimized.

Show comment
Hide comment
@vspinu

vspinu Feb 16, 2017

Member

It's a limitation of the unit parsing mechanism. Fractional units are not recognized yet. It will be fixed on the before the next release.

Member

vspinu commented Feb 16, 2017

It's a limitation of the unit parsing mechanism. Fractional units are not recognized yet. It will be fixed on the before the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment