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

Efficient instant creation functions #365

Closed
hadley opened this Issue Dec 14, 2015 · 10 comments

Comments

Projects
None yet
2 participants
@hadley
Member

hadley commented Dec 14, 2015

i.e. if I have year, month, and day, how do I efficiently create a date (or date time)?

@vspinu

This comment has been minimized.

Member

vspinu commented Dec 15, 2015

ISOdate and ISOdatetime is what you are looking for. Not the most user friendly names though.

@vspinu vspinu added the question label Dec 15, 2015

@vspinu

This comment has been minimized.

Member

vspinu commented Dec 15, 2015

How about renaming them into make_date and make_datetime? We already have make_difftime to overcome the existent base::difftime constructor.

Even better, we can use date and datetime and fall back to base::date when the function is called with no arguments.

@hadley

This comment has been minimized.

Member

hadley commented Dec 15, 2015

Sorry, the key was "efficient" - ISOdatetime() works by pasting together the values and then parsing. That's clearly a suboptimal approach. I like make_datetime(), but I think they should raise an error when called without arguments (since we already have now() and it's better for each function to do one job well)

@vspinu

This comment has been minimized.

Member

vspinu commented Dec 15, 2015

the key was "efficient" - ISOdatetime()

That's pretty efficient. Any other operation with dates will likely be much more time consuming. We can think of implementing it from scratch but I would like to see a use case when the current situation would be a bottleneck and the speed improvements would actually matter.

(somewhat related #324)

@hadley

This comment has been minimized.

Member

hadley commented Dec 15, 2015

I'd say ISODateTime is at least 100x slower than it could be. It's definitely a problem when you're working with data frame with millions of row.

We could do this much more efficiently in C.

@vspinu

This comment has been minimized.

Member

vspinu commented Dec 16, 2015

I'd say ISODateTime is at least 100x slower than it could be.

Oh. You are right. I was confused. I though that ISOdate and ISOdatetime are our functions and they are based on our C parser.

Indeed, they are very slow and I actually replaced myself all occurrences of ISOdatetime in our code a year or so ago. We already can get 100x speedup with our fast_strptime function.

@hadley

This comment has been minimized.

Member

hadley commented Dec 16, 2015

Yeah, and it should be even faster if we don't paste and parse and instead create the time object in C directly.

vspinu added a commit that referenced this issue Dec 16, 2015

@vspinu

This comment has been minimized.

Member

vspinu commented Dec 16, 2015

Hm. I have added fast_datetime. Unfortunately pasting takes 99.9% of the time so it's not a great improvement overall. I am afraid we will have to do it directly.

@vspinu

This comment has been minimized.

Member

vspinu commented Dec 25, 2015

I have ported make_datetime to C. it resulted in 100x speedup indeed.

@hadley

This comment has been minimized.

Member

hadley commented Dec 27, 2015

Nice!

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