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.

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

REF: remove BlockManager.set #33347

Merged
merged 14 commits into from
Apr 17, 2020
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
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
8 changes: 4 additions & 4 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -2830,21 +2830,21 @@ def _setitem_frame(self, key, value):
self._check_setitem_copy()
self._where(-key, value, inplace=True)

def _iset_item(self, loc: int, value):
def _iset_item(self, loc: int, value, clear: bool = True):
self._ensure_valid_index(value)

# technically _sanitize_column expects a label, not a position,
# but the behavior is the same as long as we pass broadcast=False
value = self._sanitize_column(loc, value, broadcast=False)
NDFrame._iset_item(self, loc, value)
NDFrame._iset_item(self, loc, value, clear=clear)

# check if we are modifying a copy
# try to set first as we want an invalid
# value exception to occur first
if len(self):
self._check_setitem_copy()

def _set_item(self, key, value):
def _set_item(self, key, value, clear: bool = True):
"""
Add series to DataFrame in specified column.

Expand All @@ -2856,7 +2856,7 @@ def _set_item(self, key, value):
"""
self._ensure_valid_index(value)
value = self._sanitize_column(key, value)
NDFrame._set_item(self, key, value)
NDFrame._set_item(self, key, value, clear=clear)

# check if we are modifying a copy
# try to set first as we want an invalid
Expand Down
19 changes: 13 additions & 6 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -3217,7 +3217,7 @@ def _maybe_cache_changed(self, item, value) -> None:
"""
The object has called back to us saying maybe it has changed.
"""
self._mgr.set(item, value)
NDFrame._set_item(self, item, value, clear=False)
Copy link
Member

Choose a reason for hiding this comment

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

for my edification, why NDFrame?

Copy link
Member Author

Choose a reason for hiding this comment

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

because we dont want to go through the DataFrame._set_item method

Copy link
Member

Choose a reason for hiding this comment

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

are double underscore methods appropriate here for intraclass method calls? https://docs.python.org/3/tutorial/classes.html?highlight=mangle#private-variables


@property
def _is_cached(self) -> bool_t:
Expand Down Expand Up @@ -3589,13 +3589,20 @@ def _slice(self: FrameOrSeries, slobj: slice, axis=0) -> FrameOrSeries:
result._set_is_copy(self, copy=is_copy)
return result

def _iset_item(self, loc: int, value) -> None:
def _iset_item(self, loc: int, value, clear: bool_t = True) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

where do we not clear?

Copy link
Member Author

Choose a reason for hiding this comment

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

we get here via NDFrame._maybe_cache_changed.

These are pretty spaghetti-ish, looking forward to cleaning them out

self._mgr.iset(loc, value)
self._clear_item_cache()
if clear:
self._clear_item_cache()

def _set_item(self, key, value) -> None:
self._mgr.set(key, value)
self._clear_item_cache()
def _set_item(self, key, value, clear: bool_t = True) -> None:
try:
loc = self._info_axis.get_loc(key)
except KeyError:
# This item wasn't present, just insert at end
self._mgr.insert(len(self._info_axis), key, value)
return

NDFrame._iset_item(self, loc, value, clear=clear)

def _set_is_copy(self, ref, copy: bool_t = True) -> None:
if not copy:
Expand Down
18 changes: 0 additions & 18 deletions pandas/core/internals/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -996,24 +996,6 @@ def idelete(self, indexer):
)
self._rebuild_blknos_and_blklocs()

def set(self, item: Label, value):
"""
Set new item in-place.

Notes
-----
Does not consolidate.
Adds new Block if not contained in the current items Index.
"""
try:
loc = self.items.get_loc(item)
except KeyError:
# This item wasn't present, just insert at end
self.insert(len(self.items), item, value)
return

self.iset(loc, value)

def iset(self, loc: Union[int, slice, np.ndarray], value):
"""
Set new item in-place. Does not consolidate. Adds new Block if not
Expand Down
50 changes: 26 additions & 24 deletions pandas/tests/internals/test_internals.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,8 @@ def test_iget(self):
def test_set(self):
mgr = create_mgr("a,b,c: int", item_shape=(3,))

mgr.set("d", np.array(["foo"] * 3))
mgr.set("b", np.array(["bar"] * 3))
mgr.insert(len(mgr.items), "d", np.array(["foo"] * 3))
mgr.iset(1, np.array(["bar"] * 3))
tm.assert_numpy_array_equal(mgr.iget(0).internal_values(), np.array([0] * 3))
tm.assert_numpy_array_equal(
mgr.iget(1).internal_values(), np.array(["bar"] * 3, dtype=np.object_)
Expand All @@ -354,22 +354,22 @@ def test_set(self):
)

def test_set_change_dtype(self, mgr):
mgr.set("baz", np.zeros(N, dtype=bool))
mgr.insert(len(mgr.items), "baz", np.zeros(N, dtype=bool))

mgr.set("baz", np.repeat("foo", N))
mgr.iset(mgr.items.get_loc("baz"), np.repeat("foo", N))
idx = mgr.items.get_loc("baz")
assert mgr.iget(idx).dtype == np.object_

mgr2 = mgr.consolidate()
mgr2.set("baz", np.repeat("foo", N))
mgr2.iset(mgr2.items.get_loc("baz"), np.repeat("foo", N))
idx = mgr2.items.get_loc("baz")
assert mgr2.iget(idx).dtype == np.object_

mgr2.set("quux", tm.randn(N).astype(int))
mgr2.insert(len(mgr2.items), "quux", tm.randn(N).astype(int))
idx = mgr2.items.get_loc("quux")
assert mgr2.iget(idx).dtype == np.int_

mgr2.set("quux", tm.randn(N))
mgr2.iset(mgr2.items.get_loc("quux"), tm.randn(N))
assert mgr2.iget(idx).dtype == np.float_

def test_copy(self, mgr):
Expand Down Expand Up @@ -496,9 +496,9 @@ def _compare(old_mgr, new_mgr):

# convert
mgr = create_mgr("a,b,foo: object; f: i8; g: f8")
mgr.set("a", np.array(["1"] * N, dtype=np.object_))
mgr.set("b", np.array(["2."] * N, dtype=np.object_))
mgr.set("foo", np.array(["foo."] * N, dtype=np.object_))
mgr.iset(0, np.array(["1"] * N, dtype=np.object_))
mgr.iset(1, np.array(["2."] * N, dtype=np.object_))
mgr.iset(2, np.array(["foo."] * N, dtype=np.object_))
new_mgr = mgr.convert(numeric=True)
assert new_mgr.iget(0).dtype == np.int64
assert new_mgr.iget(1).dtype == np.float64
Expand All @@ -509,9 +509,9 @@ def _compare(old_mgr, new_mgr):
mgr = create_mgr(
"a,b,foo: object; f: i4; bool: bool; dt: datetime; i: i8; g: f8; h: f2"
)
mgr.set("a", np.array(["1"] * N, dtype=np.object_))
mgr.set("b", np.array(["2."] * N, dtype=np.object_))
mgr.set("foo", np.array(["foo."] * N, dtype=np.object_))
mgr.iset(0, np.array(["1"] * N, dtype=np.object_))
mgr.iset(1, np.array(["2."] * N, dtype=np.object_))
mgr.iset(2, np.array(["foo."] * N, dtype=np.object_))
new_mgr = mgr.convert(numeric=True)
assert new_mgr.iget(0).dtype == np.int64
assert new_mgr.iget(1).dtype == np.float64
Expand Down Expand Up @@ -599,11 +599,11 @@ def test_interleave_dtype(self, mgr_string, dtype):
assert mgr.as_array().dtype == "object"

def test_consolidate_ordering_issues(self, mgr):
mgr.set("f", tm.randn(N))
mgr.set("d", tm.randn(N))
mgr.set("b", tm.randn(N))
mgr.set("g", tm.randn(N))
mgr.set("h", tm.randn(N))
mgr.iset(mgr.items.get_loc("f"), tm.randn(N))
mgr.iset(mgr.items.get_loc("d"), tm.randn(N))
mgr.iset(mgr.items.get_loc("b"), tm.randn(N))
mgr.iset(mgr.items.get_loc("g"), tm.randn(N))
mgr.iset(mgr.items.get_loc("h"), tm.randn(N))

# we have datetime/tz blocks in mgr
cons = mgr.consolidate()
Expand Down Expand Up @@ -641,7 +641,7 @@ def test_get_numeric_data(self):
"str: object; bool: bool; obj: object; dt: datetime",
item_shape=(3,),
)
mgr.set("obj", np.array([1, 2, 3], dtype=np.object_))
mgr.iset(5, np.array([1, 2, 3], dtype=np.object_))

numeric = mgr.get_numeric_data()
tm.assert_index_equal(
Expand All @@ -653,7 +653,7 @@ def test_get_numeric_data(self):
)

# Check sharing
numeric.set("float", np.array([100.0, 200.0, 300.0]))
numeric.iset(numeric.items.get_loc("float"), np.array([100.0, 200.0, 300.0]))
tm.assert_almost_equal(
mgr.iget(mgr.items.get_loc("float")).internal_values(),
np.array([100.0, 200.0, 300.0]),
Expand All @@ -663,7 +663,9 @@ def test_get_numeric_data(self):
tm.assert_index_equal(
numeric.items, pd.Index(["int", "float", "complex", "bool"])
)
numeric2.set("float", np.array([1000.0, 2000.0, 3000.0]))
numeric2.iset(
numeric2.items.get_loc("float"), np.array([1000.0, 2000.0, 3000.0])
)
tm.assert_almost_equal(
mgr.iget(mgr.items.get_loc("float")).internal_values(),
np.array([100.0, 200.0, 300.0]),
Expand All @@ -675,7 +677,7 @@ def test_get_bool_data(self):
"str: object; bool: bool; obj: object; dt: datetime",
item_shape=(3,),
)
mgr.set("obj", np.array([True, False, True], dtype=np.object_))
mgr.iset(6, np.array([True, False, True], dtype=np.object_))

bools = mgr.get_bool_data()
tm.assert_index_equal(bools.items, pd.Index(["bool"]))
Expand All @@ -684,15 +686,15 @@ def test_get_bool_data(self):
bools.iget(bools.items.get_loc("bool")).internal_values(),
)

bools.set("bool", np.array([True, False, True]))
bools.iset(0, np.array([True, False, True]))
tm.assert_numpy_array_equal(
mgr.iget(mgr.items.get_loc("bool")).internal_values(),
np.array([True, False, True]),
)

# Check sharing
bools2 = mgr.get_bool_data(copy=True)
bools2.set("bool", np.array([False, True, False]))
bools2.iset(0, np.array([False, True, False]))
tm.assert_numpy_array_equal(
mgr.iget(mgr.items.get_loc("bool")).internal_values(),
np.array([True, False, True]),
Expand Down