-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: isetitem coercing df columns to object #50175
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
Conversation
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.
generally looks good, just got a suggestion to improve testing
for i, idx in enumerate(loc): | ||
arraylike = self._sanitize_column(value.iloc[:, i]) | ||
self._iset_item_mgr(idx, arraylike, inplace=False) |
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.
in the tests you've added, i
and idx
are always the same - is it possible to add a test where i
and idx
differ?
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.
as in, if I changed this to
arraylike = self._sanitize_column(value.iloc[:, i])
self._iset_item_mgr(i, arraylike, inplace=False)
then there should be a test which fails
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.
Good catch, adjusted one of the tests
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.
Looks good to me! Over to @jbrockmendel
df = DataFrame([[1, 2, 3], [4, 5, 6]]) | ||
rhs = DataFrame([[11, 12], [13, 14]], dtype="Int64") | ||
|
||
df.isetitem([0, 1], rhs) |
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.
nitpick can we make a test file for isetitem. otherwise lgtm
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.
Done
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.
LGTM
Thanks @phofl |
frame.isetitem
incorrectly castsInt64
columns to dtypeobject
#49922 (Replace xxxx with the GitHub issue number)doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.