BUG/ENH: fix stale reference caused by block modification described in #473, add block splitting logic per #158

Author: wesm

RELEASE.rst

@@ -77,6 +77,8 @@ pandas 0.6.1
     rolling_corr/rolling_cov (GH #462)
   - Fix performance regressionm in cross-sectional count in DataFrame,
     affecting DataFrame.dropna speed
+  - Column deletion in DataFrame copies no data (computes views on blocks) (GH
+    #158)
 
 **Bug fixes**
 
@@ -100,6 +102,9 @@ pandas 0.6.1
     output type, handle tuple return values and other things that were breaking
     (GH #465)
   - Handle floating point index values in HDFStore (GH #454)
+  - Fixed stale column reference bug (cached Series object) caused by type
+    change / item deletion in DataFrame (GH #473)
+
 
 Thanks
 ------

bench/bench_dense_to_sparse.py

@@ -0,0 +1,15 @@
+from pandas import *
+
+K = 100
+N = 100000
+rng = DateRange('1/1/2000', periods=N, offset=datetools.Minute())
+
+rng2 = np.asarray(rng).astype('M8').astype('i8')
+
+series = {}
+for i in range(1, K + 1):
+    data = np.random.randn(N)[:-i]
+    this_rng = rng2[:-i]
+    data[100:] = np.nan
+    series[i] = SparseSeries(data, index=this_rng)
+

pandas/core/internals.py

@@ -160,6 +160,40 @@ def delete(self, item):
         new_values = np.delete(self.values, loc, 0)
         return make_block(new_values, new_items, self.ref_items)
 
+    def split_block_at(self, item):
+        """
+        Split block around given column, for "deleting" a column without
+        having to copy data by returning views on the original array
+
+        Returns
+        -------
+        leftb, rightb : (Block or None, Block or None)
+        """
+        loc = self.items.get_loc(item)
+
+        if len(self.items) == 1:
+            # no blocks left
+            return None, None
+
+        if loc == 0:
+            # at front
+            left_block = None
+            right_block = make_block(self.values[1:], self.items[1:].copy(),
+                                      self.ref_items)
+        elif loc == len(self.values) - 1:
+            # at back
+            left_block = make_block(self.values[:-1], self.items[:-1].copy(),
+                                    self.ref_items)
+            right_block = None
+        else:
+            # in the middle
+            left_block = make_block(self.values[:loc],
+                                    self.items[:loc].copy(), self.ref_items)
+            right_block = make_block(self.values[loc + 1:],
+                                     self.items[loc + 1:].copy(), self.ref_items)
+
+        return left_block, right_block
+
     def fillna(self, value):
         new_values = self.values.copy()
         mask = common.isnull(new_values.ravel())
@@ -573,13 +607,14 @@ def _delete_from_block(self, i, item):
         """
         Delete and maybe remove the whole block
         """
-        block = self.blocks[i]
-        newb = block.delete(item)
+        block = self.blocks.pop(i)
+        new_left, new_right = block.split_block_at(item)
 
-        if len(newb.ref_locs) == 0:
-            self.blocks.pop(i)
-        else:
-            self.blocks[i] = newb
+        if new_left is not None:
+            self.blocks.append(new_left)
+
+        if new_right is not None:
+            self.blocks.append(new_right)
 
     def _add_new_block(self, item, value):
         # Do we care about dtype at the moment?

pandas/tests/test_frame.py

@@ -3879,6 +3879,17 @@ def test_idxmax(self):
 
         self.assertRaises(Exception, frame.idxmax, axis=2)
 
+    def test_stale_cached_series_bug_473(self):
+        Y = DataFrame(np.random.random((4, 4)), index=('a', 'b','c','d'),
+                      columns=('e','f','g','h'))
+        repr(Y)
+        Y['e'] = Y['e'].astype('object')
+        Y['g']['c'] = np.NaN
+        repr(Y)
+        result = Y.sum()
+        exp = Y['g'].sum()
+        self.assert_(isnull(Y['g']['c']))
+
 class TestDataFrameJoin(unittest.TestCase):
 
     def setUp(self):

pandas/tests/test_internals.py

@@ -138,6 +138,24 @@ def test_delete(self):
 
         self.assertRaises(Exception, self.fblock.delete, 'b')
 
+    def test_split_block_at(self):
+        left, right = self.fblock.split_block_at('a')
+        self.assert_(left is None)
+        self.assert_(np.array_equal(right.items, ['c', 'e']))
+
+        left, right = self.fblock.split_block_at('c')
+        self.assert_(np.array_equal(left.items, ['a']))
+        self.assert_(np.array_equal(right.items, ['e']))
+
+        left, right = self.fblock.split_block_at('e')
+        self.assert_(np.array_equal(left.items, ['a', 'c']))
+        self.assert_(right is None)
+
+        bblock = get_bool_ex(['f'])
+        left, right = bblock.split_block_at('f')
+        self.assert_(left is None)
+        self.assert_(right is None)
+
     def test_get(self):
         pass