You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "ion-elgreco (via GitHub)" <gi...@apache.org> on 2024/10/13 09:18:37 UTC
[PR] refactor: dataframe join params [datafusion-python]
ion-elgreco opened a new pull request, #912:
URL: https://github.com/apache/datafusion-python/pull/912
# Which issue does this PR close?
- related https://github.com/apache/datafusion-python/issues/875
# Rationale for this change
Many times join keys are the same in both dataframes, so a simple on suffices. Also join_keys are deprecated over left_on and right_on.
# What changes are included in this PR?
- marks join_keys as deprecated
- introduces on, left_on, right_on parameters
- adds function overloads
# Are there any user-facing changes?
- new params added, join_keys deprecated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: dataframe `join` params [datafusion-python]
Posted by "ion-elgreco (via GitHub)" <gi...@apache.org>.
ion-elgreco commented on PR #912:
URL: https://github.com/apache/datafusion-python/pull/912#issuecomment-2427329120
@timsaucer Ah my bad, we need to add this at the end of the function, then it should run:
```
if isinstance(left_on,str):
left_on=[left_on]
if isinstance(right_on,str):
right_on=[right_on]
```
Can't add it now myself to test :(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: dataframe `join` params [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #912:
URL: https://github.com/apache/datafusion-python/pull/912#issuecomment-2464525604
Thanks for the update to fix the CI. We talked on discord and it brought up another issue - this is a breaking change for anyone who is currently doing a join without specifying the `on=` keyword. So we either need to make a change such that the old method of `df1.join(df2, (["col_a"], ["col_b"]))` will still work *OR* we have to make it clear that we've made a breaking API change and all users will need to update every instance of `join` in their code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: dataframe `join` params [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #912:
URL: https://github.com/apache/datafusion-python/pull/912#issuecomment-2464643683
Ok, I made the adjustment. Assuming CI passes I'll merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: dataframe `join` params [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #912:
URL: https://github.com/apache/datafusion-python/pull/912#issuecomment-2427310928
It looks like we do have some problems in pytests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: dataframe `join` params [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #912:
URL: https://github.com/apache/datafusion-python/pull/912#issuecomment-2426747039
Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: dataframe `join` params [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer merged PR #912:
URL: https://github.com/apache/datafusion-python/pull/912
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: dataframe `join` params [datafusion-python]
Posted by "ion-elgreco (via GitHub)" <gi...@apache.org>.
ion-elgreco commented on PR #912:
URL: https://github.com/apache/datafusion-python/pull/912#issuecomment-2425698645
> This looks like a good improvement.
Any other changes still needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: dataframe `join` params [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on code in PR #912:
URL: https://github.com/apache/datafusion-python/pull/912#discussion_r1798320329
##########
python/datafusion/dataframe.py:
##########
@@ -284,14 +324,41 @@ def join(
Args:
right: Other DataFrame to join with.
- join_keys: Tuple of two lists of column names to join on.
+ on: Column names to join on in both dataframes.
how: Type of join to perform. Supported types are "inner", "left",
"right", "full", "semi", "anti".
+ left_on: Join column of the left dataframe.
+ right_on: Join column of the right dataframe.
+ join_keys: Tuple of two lists of column names to join on. [Deprecated]
Review Comment:
With this addition, there are a set of mutually exclusive ways the user can call. You've done a very nice job of providing warnings when they are used incorrectly, but it would be very helpful to explain in the docstring that the user should provide either on OR both left_on and right_on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] refactor: dataframe `join` params [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #912:
URL: https://github.com/apache/datafusion-python/pull/912#issuecomment-2426746776
None that I can see. I'll run the workflow and as long as it all passes, I'll merge it later today
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org