Skip to content
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.

Sign up for GitHub

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

BUG: infer-objects raising for bytes Series #50067

Merged
merged 4 commits into from
Dec 8, 2022
+14 −1
Merged

BUG: infer-objects raising for bytes Series #50067

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2de660e
BUG: infer-objects raising for bytes Series
phofl Dec 5, 2022
4dc41e6
Merge remote-tracking branch 'upstream/main' into 49650
phofl Dec 7, 2022
50a33f6
Merge remote-tracking branch 'upstream/main' into 49650
phofl Dec 7, 2022
ac0a1d9
Merge remote-tracking branch 'upstream/main' into 49650
phofl Dec 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pandas/core/internals/blocks.py
View file
Original file line number Diff line number Diff line change
@@ -1969,6 +1969,9 @@ def convert(
attempt to cast any object types to better types return a copy of
the block (if copy = True) by definition we ARE an ObjectBlock!!!!!
"""
if self.dtype != object:
return [self]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this may need to make a copy in some cases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure?

ser = Series(["a"], dtype="object")
result = ser.infer_objects()
result.iloc[0] = "c"
print(ser)

this updates ser as well, same as the bytes version. If we add a copy, the bytes version would not update anymore

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. infer_objects in principle makes a copy in cases where it doesn't do inference. 2) I don't think that's what most users (or our internal usages) actually want. 3) ATM the ObjectBlock case that doesn't do inference doesn't make a copy, which is inconsistent with other dtypes. 4) i think adding a copy keyword makes sense xref ENH/PERF: copy=True keyword in methods where copy=False can improve perf #48141, but adding copy keywords in general got pushback recently

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can ignore this since were not consistent about it anyway

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can address in follow up


values = self.values
if values.ndim == 2:
# maybe_split ensures we only get here with values.shape[0] == 1,
12 changes: 11 additions & 1 deletion pandas/tests/series/methods/test_infer_objects.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import numpy as np

from pandas import interval_range
from pandas import (
Series,
interval_range,
)
import pandas._testing as tm


@@ -44,3 +47,10 @@ def test_infer_objects_interval(self, index_or_series):

result = obj.astype(object).infer_objects()
tm.assert_equal(result, obj)

def test_infer_objects_bytes(self):
# GH#49650
ser = Series([b"a"], dtype="bytes")
expected = ser.copy()
result = ser.infer_objects()
tm.assert_series_equal(result, expected)