-
Notifications
You must be signed in to change notification settings - Fork 144
Add join_asof/3
#1122
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
Add join_asof/3
#1122
Conversation
billylanchantin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks great! It's also been in my queue for quite a while, so I appreciate you getting it across the finish line.
Maybe as part of this PR, or as a separate one, would be nice to add other options supported by Polars:
I leave that up to you. I'd be happy to merge this essentially as is, then you can add the additional options as a follow-on if you like.
My only quibble is that I find the explanation of by a bit lacking. (The Polars docs are the real culprit here.) I had to do a few tests in console of that option before I understood the intended behavior.
|
Thank you for the review!
I think adding them as a follow-on is easier for everyone.
I would agree... Perhaps, I could add a few more tests showing in more depth how |
Co-authored-by: Billy Lanchantin <[email protected]>
billylanchantin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went a little overboard and attempted a rewrite of the Examples section.
After rereading, I found the examples that Polars chose a little confusing. I think my brain just doesn't parse the dates fast enough to follow super easily.
My intended audience is someone who knows what a left join is but hasn't encountered the "asof" join before. LMK if you think this is better. I'm also happy to go with something else!
Co-authored-by: Billy Lanchantin <[email protected]>
|
It seems there is nothing left to add to this PR. Or would you like me to change something else? |
|
No this is awesome, thanks again! ❤️ |
|
❤️💜 |
|
Yeah thanks to @jeregrine as well for getting this started! The "asof" join has been useful to me on a number of occasions and I'm really glad we can incorporate it :) |
I have added some changes on top of #1080:
:onto be a single column (join_asofcurrently works only with a single column)Cargo.toml; reduce boilerplate of:on/:bynormalisation).Maybe as part of this PR, or as a separate one, would be nice to add other options supported by Polars:
:tolerance:allow_eq:check_sortedness