Repository: pandas-dev/pandas
Pull request: BUG-24971 copying blocks also considers ndim
- [X] closes #24971 
- [X] tests added / passed
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [X] whatsnew entry

The following gives a series containing [1] instead of 1
```
>>> pd.Series(pd.Categorical('A', categories=['A', 'B'])).replace({'A': 1, 'B': 2})
0    [1]
dtype: object
```
This bug occurs because in the process of copying the original categorical block (which is needed as the operation is not inplace), the constructor class for the new object defaults to `ObjectBlock`, whose constructor has a default `ndim` of 2. This PR alters the block `copy` function to specify that the newly constructed block should have the same `ndim` as the block being copied.

Problem statement:
Categorical.replace returns unexpected dimensions for length 1 Series
The shapes and types of the first two are unexpected. Should they be? Just upgraded to pandas 0.24.0.

```python
pd.Series(pd.Categorical('A', categories=['A', 'B'])).replace({'A': 1, 'B': 2})
# Out[30]:
# 0    [1]
# dtype: object

pd.Series(pd.Categorical(('A', ), categories=['A', 'B'])).replace({'A': 1, 'B': 2})
# Out[31]:
# 0    [1]
# dtype: object

pd.Series(pd.Categorical(('A', 'B'), categories=['A', 'B'])).replace({'A': 1, 'B': 2})
# Out[32]:
# 0    1
# 1    2
# dtype: int64
```

I expect the first to be an int64 dtype with a shape of (1,). They return a Series with shape (1, 1) and dtype object.

<details>
INSTALLED VERSIONS
------------------
commit: None
python: 3.7.0.final.0
python-bits: 64
OS: Linux
OS-release: 4.4.0-116-generic
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8

pandas: 0.24.0
pytest: 4.1.1
pip: 18.1
setuptools: 40.2.0
Cython: None
numpy: 1.15.4
scipy: 1.2.0
pyarrow: None
xarray: None
IPython: 7.2.0
sphinx: None
patsy: None
dateutil: 2.6.1
pytz: 2018.7
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: 3.0.2
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml.etree: None
bs4: None
html5lib: None
sqlalchemy: 1.2.15
pymysql: None
psycopg2: 2.7.6.1 (dt dec pq3 ext lo64)
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None
gcsfs: None
</details>

Hints from issue discussion:
@jseabold Thanks for the report!

What version were you using before upgrading? I see this buggy behaviour in 0.23.4 as well.
> What version were you using before upgrading?

I was using 0.23.4. I upgraded to see if another bug was fixed (it was), which revealed this one for me. Don't know anything about whether this code would have worked in an older version.
OK, just to know whether it was a regression or not. Of course it is still a clear bug, but if it was a regression compared to 0.23.4, we should have tried to give it priority.

Commit message: BUG-24971 copying blocks also considers ndim

Patch to review:
diff --git a/doc/source/whatsnew/v0.24.2.rst b/doc/source/whatsnew/v0.24.2.rst
index 4fcde7769b362..8d62994c8181d 100644
--- a/doc/source/whatsnew/v0.24.2.rst
+++ b/doc/source/whatsnew/v0.24.2.rst
@@ -66,7 +66,7 @@ Bug Fixes
 
 **Categorical**
 
--
+- Bug where a copy of a categorical could have the wrong dimensions (:issue:`24971`)
 -
 -
 
diff --git a/pandas/core/internals/blocks.py b/pandas/core/internals/blocks.py
index 4e2c04dba8b04..d23a2a1224e16 100644
--- a/pandas/core/internals/blocks.py
+++ b/pandas/core/internals/blocks.py
@@ -738,7 +738,7 @@ def copy(self, deep=True):
         values = self.values
         if deep:
             values = values.copy()
-        return self.make_block_same_class(values)
+        return self.make_block_same_class(values, ndim=self.ndim)
 
     def replace(self, to_replace, value, inplace=False, filter=None,
                 regex=False, convert=True):
diff --git a/pandas/tests/series/test_constructors.py b/pandas/tests/series/test_constructors.py
index d92ca48751d0a..17f522fe173e3 100644
--- a/pandas/tests/series/test_constructors.py
+++ b/pandas/tests/series/test_constructors.py
@@ -14,7 +14,7 @@
 from pandas.compat import PY36, long, lrange, range, zip
 
 from pandas.core.dtypes.common import (
-    is_categorical_dtype, is_datetime64tz_dtype)
+    is_categorical_dtype, is_datetime64tz_dtype, is_list_like)
 
 import pandas as pd
 from pandas import (
@@ -532,6 +532,11 @@ def test_constructor_copy(self):
             assert x[0] == 2.
             assert y[0] == 1.
 
+    def test_copy_categorical_ndim(self):
+        # GH 24971
+        s = Series(Categorical(['A'], categories=['A']))
+        assert not is_list_like(s.replace({'A': 1})[0])
+
     @pytest.mark.parametrize(
         "index",
         [


Code review comments:
diff_hunk: @@ -66,7 +66,7 @@ Bug Fixes
 
 **Categorical**
 
--
+- Bug where a copy of a categorical could have the wrong dimensions (:issue:`24971`)
path: doc/source/whatsnew/v0.24.2.rst
text: @user1:
can you reword to make this more clear. mentione this is on a Series of categorical dtype. The dimenension issue is an internal one. You want to emphasize the user visible effects.

@user1:
this is visible when you call ``.replace()``

diff_hunk: @@ -532,6 +532,11 @@ def test_constructor_copy(self):
             assert x[0] == 2.
             assert y[0] == 1.
 
+    def test_copy_categorical_ndim(self):
+        # GH 24971
+        s = Series(Categorical(['A'], categories=['A']))
+        assert not is_list_like(s.replace({'A': 1})[0])
+
path: pandas/tests/series/test_constructors.py
text: @user1:
this should go with the replace tests

diff_hunk: @@ -532,6 +532,11 @@ def test_constructor_copy(self):
             assert x[0] == 2.
             assert y[0] == 1.
 
+    def test_copy_categorical_ndim(self):
+        # GH 24971
+        s = Series(Categorical(['A'], categories=['A']))
+        assert not is_list_like(s.replace({'A': 1})[0])
path: pandas/tests/series/test_constructors.py
text: @user1:
use assert_series_equal