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

Convert Period to numeric #420

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

Comments

Projects
None yet
2 participants
@etiennebr

etiennebr commented Jun 9, 2016

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

This comment has been minimized.

etiennebr commented Jun 9, 2016

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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 in 7a87c02 Jun 25, 2016

@vspinu

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

etiennebr commented Jun 29, 2016

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

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