-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: Raise TypeError when joining with non-DataFrame using 'on=' (GH#61434) #61454
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
base: main
Are you sure you want to change the base?
Conversation
78fbc2c
to
b604714
Compare
All checks passed except the Pyodide build, which failed due to a rate-limit (HTTP 429). The failure seems unrelated to this PR. A rerun should resolve it |
b604714
to
71eb2e7
Compare
if isinstance(other, Iterable) and not isinstance( | ||
other, (DataFrame, Series, str, bytes, bytearray) | ||
): |
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.
if isinstance(other, Iterable) and not isinstance( | |
other, (DataFrame, Series, str, bytes, bytearray) | |
): | |
if is_list_like(other): |
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.
Also, this probably should be done in merge
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.
@mroeschke - is_list_like will return True on a DataFrame. We only want to enter this block on a potential sequence of DataFrame/Series.
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.
Ah OK ignore my suggestion then. But I believe this check should still be done in merge
probably
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.
Ah OK ignore my suggestion then. But I believe this check should still be done in
merge
probably
ValueError
is raised in join()
before the call reaches merge
when on
is specified. Would you prefer that I let these inputs flow into merge
and move the check there for consistency?
71eb2e7
to
6789fb6
Compare
6789fb6
to
c9ac591
Compare
Closes GH#61434
What does this PR change?
When using
DataFrame.join()
with theon
parameter, passing an invalid object like adict
,int
, or third-party DataFrame previously resulted in unclear internal errors.This PR adds a minimal type check that raises a clear
TypeError
whenother
is not aDataFrame
,Series
, or a list of such objects. Valid list-based joins withouton
remain unaffected.Checklist
pre-commit run --all-files
doc/source/whatsnew/v3.0.0.rst