Repository: pandas-dev/pandas
Pull request: TST: add tests to validate margin results for pivot (#25815)
- [x ] closes #25815
- [x ] tests added / passed
- [x ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Problem statement:
wrong aggregated results when pivot_table index is ordered categorical data 
#### Code Sample, a copy-pastable example if possible


```python
import pandas as pd
import numpy as np

_c = np.arange(5,0,-1)

df = pd.DataFrame(np.stack([_c]*3).T, columns=['A', 'B', 'C'])

df["D"] = pd.Series(["A","B","B","A", "A"])

df["E"] = pd.cut(x=df["A"],bins=[0,3,6,10])

print(pd.pivot_table(df, index='E', columns=['D'], values="B", aggfunc="sum", margins=True))
```
``` ipython
Output is

D       A  B  All
E                
(0, 3]  3  3    9
(3, 6]  5  4    6
All     8  7   15
```
#### Problem description
the data is as follows:
``` Ipython
	A	B	C	D	E
0	5	5	5	A	(3, 6]
1	4	4	4	B	(3, 6]
2	3	3	3	B	(0, 3]
3	2	2	2	A	(0, 3]
4	1	1	1	A	(0, 3]
```
Margin calculation results are in wrong order
(0, 3] should have a sum value of 6,
(3, 6] should have a sum value of 9

If I dont use column argument, results are correct.
``` Ipython
B
E	
(0, 3]	6
(3, 6]	9
(6, 10]	0
All	15
```

I've tried both .23.4 and .24.2 . They have the same results.

#### Expected Output
``` Ipython
D       A  B  All
E                
(0, 3]  3  3    6
(3, 6]  5  4    9
All     8  7   15
```
#### Output of ``pd.show_versions()``

<details>

INSTALLED VERSIONS
------------------
commit: None
python: 3.7.1.final.0
python-bits: 64
OS: Windows
OS-release: 10
machine: AMD64
processor: Intel64 Family 6 Model 142 Stepping 9, GenuineIntel
byteorder: little
LC_ALL: None
LANG: zh_CN.GBK@cjknarrow
LOCALE: None.None

pandas: 0.23.4
pytest: 4.0.2
pip: 18.1
setuptools: 40.6.3
Cython: 0.29.2
numpy: 1.15.4
scipy: 1.1.0
pyarrow: None
xarray: None
IPython: 7.2.0
sphinx: 1.8.2
patsy: 0.5.1
dateutil: 2.7.5
pytz: 2018.7
blosc: None
bottleneck: 1.2.1
tables: 3.4.4
numexpr: 2.6.8
feather: None
matplotlib: 3.0.2
openpyxl: 2.5.12
xlrd: 1.2.0
xlwt: 1.3.0
xlsxwriter: 1.1.2
lxml: 4.2.5
bs4: 4.6.3
html5lib: 1.0.1
sqlalchemy: 1.2.15
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None

</details>

Hints from issue discussion:
check on 0.24.2 i believe we patched this 
https://github.com/pandas-dev/pandas/issues/25814

I tried on 0.24.2. It gives me an error. It might be gone, but I cant tell.


> check on 0.24.2 i believe we patched this

```ipython
In [19]: import pandas as pd
    ...: import numpy as np
    ...:
    ...: _c = np.arange(5,0,-1)
    ...:
    ...: df = pd.DataFrame(np.stack([_c]*3).T, columns=['A', 'B', 'C'])
    ...:
    ...: df["D"] = pd.Series(["A","B","B","A", "A"])
    ...:
    ...: df["E"] = pd.cut(x=df["A"],bins=[0,3,6,10]).cat.add_categories("Miss")
    ...:
    ...:
    ...:

In [20]: df
Out[20]:
   A  B  C  D       E
0  5  5  5  A  (3, 6]
1  4  4  4  B  (3, 6]
2  3  3  3  B  (0, 3]
3  2  2  2  A  (0, 3]
4  1  1  1  A  (0, 3]

In [21]: print(pd.pivot_table(df, index='E', columns=['D'], values="B", aggfunc="
    ...: sum", margins=True))
D       A  B  All
E
(0, 3]  3  3    9
(3, 6]  5  4    6
All     8  7   15

I can bypass 
#25814  , by adding a new category.  However the results are still wrong. The above code is tested on current master.
pandas: 0.25.0.dev0+299.ge31b4f46d
I would like to take a shot at this. 
@mukundm19 Any progress yet ?
There's a dirty workaround for this. You could convert your intervals to strings. Obviously you lose all of the properties of the intervals. If you're OK with that (lets say you only show the intervals and don't operate on them afterwards) then you're golden.

You can workaround this by adding `.astype(str)` after the call to `.cut(...)`.

```py
import pandas as pd
import numpy as np

_c = np.arange(5,0,-1)

df = pd.DataFrame(np.stack([_c]*3).T, columns=['A', 'B', 'C'])

df["D"] = pd.Series(["A","B","B","A", "A"])

df["E"] = pd.cut(x=df["A"],bins=[0,3,6,10]).astype(str)

print(pd.pivot_table(df, index='E', columns=['D'], values="B", aggfunc="sum", margins=True))
```

```
D       A  B  All
E
(0, 3]  3  3    6
(3, 6]  5  4    9
All     8  7   15
```
This looks to be fixed on master now, and just could use a test:
```python
In [1]: import numpy as np; import pandas as pd; pd.__version__
Out[1]: '0.25.0.dev0+833.gad18ea35b'

In [2]: _c = np.arange(5,0,-1) 
   ...: df = pd.DataFrame(np.stack([_c]*3).T, columns=['A', 'B', 'C']) 
   ...: df["D"] = pd.Series(["A","B","B","A", "A"]) 
   ...: df["E"] = pd.cut(x=df["A"],bins=[0,3,6,10])

In [3]: df
Out[3]: 
   A  B  C  D       E
0  5  5  5  A  (3, 6]
1  4  4  4  B  (3, 6]
2  3  3  3  B  (0, 3]
3  2  2  2  A  (0, 3]
4  1  1  1  A  (0, 3]

In [4]: pd.pivot_table(df, index='E', columns=['D'], values="B", aggfunc="sum", margins=True)
Out[4]: 
D       A  B  All
E                
(0, 3]  3  3    6
(3, 6]  5  4    9
All     8  7   15
```

Commit message: TST: add tests to validate margin results for pivot (#25815)

Patch to review:
diff --git a/pandas/tests/reshape/test_pivot.py b/pandas/tests/reshape/test_pivot.py
index b497f6c3aa9b4..84df97c79e789 100644
--- a/pandas/tests/reshape/test_pivot.py
+++ b/pandas/tests/reshape/test_pivot.py
@@ -286,6 +286,25 @@ def test_pivot_with_interval_index(self, interval_values, dropna):
         expected = DataFrame({"B": 1}, index=Index(interval_values.unique(), name="A"))
         tm.assert_frame_equal(result, expected)
 
+    def test_pivot_with_interval_index_margins(self, dropna):
+        # GH 25815
+        ordered_cat = pd.IntervalIndex.from_arrays(
+            [0, 0, 1, 1], [1, 1, 2, 2])
+        df = pd.DataFrame({
+            'A': np.arange(4, 0, -1),
+            'B': ['a', 'b', 'a', 'b'],
+            'C': pd.Categorical(ordered_cat,
+                                ordered=True).sort_values(ascending=False)
+        })
+
+        pivot_tab = pd.pivot_table(data=df, index='C', columns='B', values='A',
+                                   aggfunc='sum', margins=True)
+
+        result = pivot_tab['All']
+        expected = pivot_tab.iloc[:, :-1].sum(axis=1)
+        tm.assert_series_equal(result, expected, check_dtype=False,
+                               check_names=False)
+
     def test_pass_array(self):
         result = self.data.pivot_table("D", index=self.data.A, columns=self.data.C)
         expected = self.data.pivot_table("D", index="A", columns="C")


Code review comments:
diff_hunk: @@ -286,6 +286,25 @@ def test_pivot_with_interval_index(self, interval_values, dropna):
         expected = DataFrame({"B": 1}, index=Index(interval_values.unique(), name="A"))
         tm.assert_frame_equal(result, expected)
 
+    def test_pivot_with_interval_index_margins(self, dropna):
+        # GH 25815
+        ordered_cat = pd.IntervalIndex.from_arrays(
+            [0, 0, 1, 1], [1, 1, 2, 2])
+        df = pd.DataFrame({
+            'A': np.arange(4, 0, -1),
+            'B': ['a', 'b', 'a', 'b'],
+            'C': pd.Categorical(ordered_cat,
+                                ordered=True).sort_values(ascending=False)
+        })
+
+        pivot_tab = pd.pivot_table(data=df, index='C', columns='B', values='A',
+                                   aggfunc='sum', margins=True)
+
+        result = pivot_tab['All']
+        expected = pivot_tab.iloc[:, :-1].sum(axis=1)
path: pandas/tests/reshape/test_pivot.py
text: @user1:
Can you just construct this manually? I think generating the `result` and `expected` values from the same variable can let bugs pass through undetected

diff_hunk: @@ -286,6 +286,25 @@ def test_pivot_with_interval_index(self, interval_values, dropna):
         expected = DataFrame({"B": 1}, index=Index(interval_values.unique(), name="A"))
         tm.assert_frame_equal(result, expected)
 
+    def test_pivot_with_interval_index_margins(self, dropna):
+        # GH 25815
+        ordered_cat = pd.IntervalIndex.from_arrays(
+            [0, 0, 1, 1], [1, 1, 2, 2])
+        df = pd.DataFrame({
+            'A': np.arange(4, 0, -1),
+            'B': ['a', 'b', 'a', 'b'],
+            'C': pd.Categorical(ordered_cat,
+                                ordered=True).sort_values(ascending=False)
+        })
+
+        pivot_tab = pd.pivot_table(data=df, index='C', columns='B', values='A',
+                                   aggfunc='sum', margins=True)
+
+        result = pivot_tab['All']
+        expected = pivot_tab.iloc[:, :-1].sum(axis=1)
+        tm.assert_series_equal(result, expected, check_dtype=False,
path: pandas/tests/reshape/test_pivot.py
text: @user1:
Can you remove `check_dtype` and `check_names`?

diff_hunk: @@ -286,6 +286,25 @@ def test_pivot_with_interval_index(self, interval_values, dropna):
         expected = DataFrame({"B": 1}, index=Index(interval_values.unique(), name="A"))
         tm.assert_frame_equal(result, expected)
 
+    def test_pivot_with_interval_index_margins(self, dropna):
+        # GH 25815
+        ordered_cat = pd.IntervalIndex.from_arrays(
+            [0, 0, 1, 1], [1, 1, 2, 2])
+        df = pd.DataFrame({
+            'A': np.arange(4, 0, -1),
+            'B': ['a', 'b', 'a', 'b'],
+            'C': pd.Categorical(ordered_cat,
+                                ordered=True).sort_values(ascending=False)
+        })
+
+        pivot_tab = pd.pivot_table(data=df, index='C', columns='B', values='A',
path: pandas/tests/reshape/test_pivot.py
text: @user1:
```suggestion
        pivot_tab = pd.pivot_table(df, index='C', columns='B', values='A',
```

diff_hunk: @@ -286,6 +286,25 @@ def test_pivot_with_interval_index(self, interval_values, dropna):
         expected = DataFrame({"B": 1}, index=Index(interval_values.unique(), name="A"))
         tm.assert_frame_equal(result, expected)
 
+    def test_pivot_with_interval_index_margins(self, dropna):
path: pandas/tests/reshape/test_pivot.py
text: @user1:
do we need to inject `dropna` here? I think you can just remove.