BUG: avoid unnecessary casting when unstacking index with unused levels #18460

toobaz · 2017-11-23T23:08:03Z

tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

codecov · 2017-11-24T02:49:56Z

Codecov Report

❗ No coverage uploaded for pull request base (master@de42bee). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #18460   +/-   ##
=========================================
  Coverage          ?   91.55%           
=========================================
  Files             ?      147           
  Lines             ?    48827           
  Branches          ?        0           
=========================================
  Hits              ?    44703           
  Misses            ?     4124           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`89.92% <100%> (?)`
#single	`41.69% <6.25%> (?)`

Impacted Files	Coverage Δ
pandas/core/reshape/reshape.py	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de42bee...a629b82. Read the comment docs.

jreback · 2017-11-24T19:42:28Z

related to #17886 or is that separate?

jreback · 2017-11-24T19:43:59Z

pandas/core/reshape/reshape.py

            new_names = [self.value_columns.name, self.removed_name]
            new_labels = [propagator]

-        new_labels.append(np.tile(np.arange(stride) - self.lift, width))


can you add a comment on what is going on here (e.g. the unsused bizness)

(see below)

jreback · 2017-11-24T19:45:21Z

pandas/tests/frame/test_reshape.py

+            exp_data[idces] = data
+            cols = pd.MultiIndex.from_product([[0, 1], col_level])
+            expected = pd.DataFrame(exp_data.reshape(3, 6),
+                                    index=idx_level, columns=cols)


can we have an exact expected frame and assert_frame_equal (maybe more code, but it really locks it down to the exact result).

The frame is an exact copy, but assert_frame_equal fails (two lines below) because of #18455 . So until that is fixed, I guess I can only add a check on the dtypes.

So until that is fixed, I guess I can only add a check on the dtypes.

(shall I?)

I think worthwhile to fix #18455 first actually.

@jreback : seems like #18455 won't be fixed before #18600, #18626, #18769... and the present PR (probably together with #18562) is hanging only for a workaround in a test. Still want to wait?

jreback

comments

toobaz · 2017-11-24T19:53:49Z

related to #17886 or is that separate?

Separate - this fix handles specifically the case of unused levels. But I plan to look at #17886 too.

jreback

iiirc this was pretty good. pls rebase and let's see where this is.

jreback · 2018-01-14T22:19:00Z

doc/source/whatsnew/v0.22.0.txt

@@ -318,6 +318,7 @@ Sparse
 Reshaping
 ^^^^^^^^^

+- Bug in :func:`DataFrame.unstack` which casts int to float if ``columns`` is a ``MultiIndex`` with unused levels (:issue:`17845`)


move to 0.23

toobaz · 2018-01-15T13:36:12Z

@jreback rebased, I also verified that this fixes #18562 and added a test for that

jreback

looks good. style comments.

jreback · 2018-01-15T13:52:12Z

pandas/core/reshape/reshape.py

            new_names = [self.value_columns.name, self.removed_name]
            new_labels = [propagator]

-        new_labels.append(np.tile(np.arange(stride) - self.lift, width))
+        # The two indices differ iff the unstacked level had unused items.


(if and only if - but OK)

jreback · 2018-01-15T13:52:55Z

pandas/core/reshape/reshape.py

+        else:
+            # Otherwise, we just use each level item exactly once:
+            repeater = np.arange(stride) - self.lift
+        # The entire level is then just a repetition of the single chunk:


blank line here

jreback · 2018-01-15T13:53:22Z

pandas/tests/frame/test_reshape.py

@@ -560,6 +560,73 @@ def test_unstack_dtypes(self):
            assert left.shape == (3, 2)
            tm.assert_frame_equal(left, right)

+    def test_unstack_unused_levels(self):
+        # GH 17845: sliced columns of int DataFrame


can you give 1 more line of expl here

jreback · 2018-01-15T13:53:44Z

pandas/tests/frame/test_reshape.py

+        result = df.unstack()
+        expected = pd.DataFrame(np.concatenate([block * 2, block * 2 - 1],
+                                               axis=1),
+                                columns=idx)


you are not comparing here?

jreback · 2018-01-15T13:54:23Z

pandas/tests/frame/test_reshape.py

+        expected.columns = MultiIndex.from_product([expected.columns, ['I']],
+                                                   names=[None, 'C'])
+        expected.index = expected.index.droplevel('C')
+        assert_frame_equal(result, expected)


use tm.assert_frame_equal for consistentcy

the following test uses assert_frame_equal, but if that's a leftover, OK

jreback · 2018-01-15T13:56:26Z

pandas/tests/frame/test_reshape.py

+
+    @pytest.mark.parametrize("cols", [['A', 'C'], slice(None)])
+    def test_unstack_unused_level(self, cols):
+        # GH 18562 : unused labels on the unstacked level


I believe you had 2 cases for #18562 does this cover both?

yes, they are the two "cols" values

closes pandas-dev#17845 closes pandas-dev#18562

toobaz · 2018-01-15T19:54:12Z

@jreback ping

jreback · 2018-01-16T00:18:52Z

thanks @toobaz

TomAugspurger · 2018-01-17T12:57:52Z

FYI, this caused a 25-30% slowdown in this ASV: http://pandas.pydata.org/speed/pandas/#reshape.SparseIndex.time_unstack

toobaz · 2018-01-17T14:03:54Z

I guess it's the remove_unused_levels call. I can see if (and how) it can be ran on specific levels only.

jreback · 2018-01-17T16:43:24Z

can u create an issue to track this

toobaz force-pushed the slice_unstack_unused branch from 7dd32be to 99a5dce Compare November 23, 2017 23:08

jreback added Dtype Conversions Unexpected or buggy dtype conversions Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Nov 24, 2017

jreback reviewed Nov 24, 2017

View reviewed changes

jreback requested changes Nov 24, 2017

View reviewed changes

toobaz mentioned this pull request Nov 26, 2017

Construction of Series from dict containing NaN as key #18496

Merged

4 tasks

toobaz force-pushed the slice_unstack_unused branch from 99a5dce to 26a9968 Compare December 18, 2017 13:24

jreback requested changes Jan 14, 2018

View reviewed changes

toobaz force-pushed the slice_unstack_unused branch from 26a9968 to 2dc6cab Compare January 15, 2018 10:32

jreback requested changes Jan 15, 2018

View reviewed changes

BUG: fix unstacking with unused levels in columns/unstacked index level

a629b82

closes pandas-dev#17845 closes pandas-dev#18562

toobaz force-pushed the slice_unstack_unused branch from 2dc6cab to a629b82 Compare January 15, 2018 14:17

jreback added this to the 0.23.0 milestone Jan 16, 2018

jreback approved these changes Jan 16, 2018

View reviewed changes

jreback merged commit 7208610 into pandas-dev:master Jan 16, 2018

toobaz deleted the slice_unstack_unused branch January 16, 2018 04:47

TomAugspurger mentioned this pull request Jan 17, 2018

Unstack performance regression #19289

Closed

deisdenis mentioned this pull request Aug 2, 2018

BUG: unstack() does not always sort index in 0.23 #21675

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: avoid unnecessary casting when unstacking index with unused levels #18460

BUG: avoid unnecessary casting when unstacking index with unused levels #18460

toobaz commented Nov 23, 2017 •

edited

Loading

codecov bot commented Nov 24, 2017 •

edited

Loading

jreback commented Nov 24, 2017

jreback Nov 24, 2017

toobaz Dec 18, 2017

jreback Nov 24, 2017

toobaz Nov 24, 2017

toobaz Nov 24, 2017

jreback Nov 24, 2017

toobaz Dec 18, 2017

jreback left a comment

toobaz commented Nov 24, 2017 •

edited

Loading

jreback left a comment

jreback Jan 14, 2018

toobaz commented Jan 15, 2018

jreback left a comment

jreback Jan 15, 2018

toobaz Jan 15, 2018

jreback Jan 15, 2018

jreback Jan 15, 2018

jreback Jan 15, 2018

jreback Jan 15, 2018

toobaz Jan 15, 2018

jreback Jan 15, 2018

toobaz Jan 15, 2018

toobaz commented Jan 15, 2018

jreback commented Jan 16, 2018

TomAugspurger commented Jan 17, 2018

toobaz commented Jan 17, 2018

jreback commented Jan 17, 2018

BUG: avoid unnecessary casting when unstacking index with unused levels #18460

BUG: avoid unnecessary casting when unstacking index with unused levels #18460

Conversation

toobaz commented Nov 23, 2017 • edited Loading

codecov bot commented Nov 24, 2017 • edited Loading

Codecov Report

jreback commented Nov 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

toobaz commented Nov 24, 2017 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toobaz commented Jan 15, 2018

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toobaz commented Jan 15, 2018

jreback commented Jan 16, 2018

TomAugspurger commented Jan 17, 2018

toobaz commented Jan 17, 2018

jreback commented Jan 17, 2018

toobaz commented Nov 23, 2017 •

edited

Loading

codecov bot commented Nov 24, 2017 •

edited

Loading

toobaz commented Nov 24, 2017 •

edited

Loading