Skip to content
New issue

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

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

Already on GitHub? # to your account

Convert Period to numeric #420

Closed
etiennebr opened this issue Jun 9, 2016 · 7 comments
Closed

Convert Period to numeric #420

etiennebr opened this issue Jun 9, 2016 · 7 comments

Comments

@etiennebr
Copy link

I couldn't find the right way to do it, but it seems hard to convert Period elements to numeric.
This is the expected behavior (as.numeric on difftime)

as.numeric(as.difftime("24:00:00"), "hours")
# [1] 24
as.numeric(as.difftime("24:00:00"), "mins")
# [1] 1440

However Period objects only return the unitpart of the Period, like components getters (second, minute, ... )

as.numeric(period(24, "hours"), "mins")
# [1] 0  # This is unexpected to me, I believe it's an error
as.numeric(period(24, "hours") + period(10, "minutes"), "mins")
# [1] 10
as.numeric(period(24, "hours"), "hours")
# [1] 24

The trick I found was to convert to duration, but I get a warning when converting Period to duration.

as.numeric(as.duration(period(10, "minutes")), "mins")
# estimate only: convert periods to intervals for accuracy
# [1] 10
as.numeric(as.duration(period(10, "minutes")), "hours")
# estimate only: convert periods to intervals for accuracy
# [1] 0.1666667
@etiennebr
Copy link
Author

Ok, I found time_length as the equivalent of as.numeric(as.duration()), but it still raises a warning.

Stepping through the function, it works like a component getter. Shouldn't the behavior be similar to a Duration object ?

I'm slightly confused between duration and period, but it's so convenient to use minutes(20) and it happens to return a period. Would it be possible to raise the coercion warning only when there actually is a loss of precision ? I think it's also unexpected that these two objects have different behaviors.

@vspinu
Copy link
Member

vspinu commented Jun 25, 2016

Sorry for coming late on this.

Periods don't have a predefined length, hence the warning.

Would it be possible to raise the coercion warning only when there actually is a loss of precision ?

Yes. This makes sense indeed.

@vspinu
Copy link
Member

vspinu commented Jun 25, 2016

as.numeric(period(24, "hours"), "mins")

[1] 0 # This is unexpected to me, I believe it's an error

This behavior is undocumented but I agree that it doesn't make sense.

@vspinu
Copy link
Member

vspinu commented Jun 25, 2016

BTW, there is period_to_seconds to convert to seconds without a warning. Checking if period could be converted exactly to specified unit would require checking if all components higher than day are zero and would introduce unnecessary inneficiency. So I am inclined to keep the warning as it is. But I will try to fix the as.numeric part.

@vspinu vspinu closed this as completed in 7a87c02 Jun 25, 2016
@vspinu
Copy link
Member

vspinu commented Jun 25, 2016

Fixing as.numeric and also removing all other estimate only warnings. Those are annoying, often unnecessary (units smaller/equal to days) and are deemed common knowledge by now.

Let me know if you see more problems of this kind. Thanks.

@vspinu
Copy link
Member

vspinu commented Jun 25, 2016

One last thing. No need to sum up peridos. You can produce those in one go:

per <- period(hours = 10, minutes = 6)
as.numeric(per, "hours")
# [1] 10.1

@etiennebr
Copy link
Author

Great @vspinu, and many thanks for the period tip !

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

No branches or pull requests

2 participants