pandas-dev / pandas

Flexible and powerful data analysis / manipulation library for Python, providing labeled data structures similar to R data.frame objects, statistical functions, and much more
https://pandas.pydata.org
BSD 3-Clause "New" or "Revised" License
43.72k stars 17.94k forks source link

BUG: MultiIndex.factorize fails if index is 0-length #57517

Open batterseapower opened 8 months ago

batterseapower commented 8 months ago

Pandas version checks

Reproducible Example

empty_ix = pd.MultiIndex.from_product([
    pd.Index([], name='a', dtype=object),
    pd.Index([], name='i', dtype='f4')
])
empty_ix.factorize() # Fails

# Names are lost
pd.MultiIndex.from_product([
    pd.Index(['a', 'b'], name='a'),
    pd.Index([0, 1, 0], name='i', dtype='f4')
]).factorize()[1].names

Issue Description

The first example should succeed, but actually it fails:

File ~/opt/conda/envs/mamba/envs/py3_2/lib/python3.11/site-packages/pandas/core/base.py:1203, in IndexOpsMixin.factorize(self, sort, use_na_sentinel)
   1199     uniques = uniques.astype(np.float32)
   1201 if isinstance(self, ABCIndex):
   1202     # preserve e.g. MultiIndex
-> 1203     uniques = self._constructor(uniques)
   1204 else:
   1205     from pandas import Index
File ~/opt/conda/envs/mamba/envs/py3_2/lib/python3.11/site-packages/pandas/core/indexes/multi.py:222, in names_compat.<locals>.new_meth(self_or_cls, *args, **kwargs)
    219 if "name" in kwargs:
    220     kwargs["names"] = kwargs.pop("name")
--> 222 return meth(self_or_cls, *args, **kwargs)
File ~/opt/conda/envs/mamba/envs/py3_2/lib/python3.11/site-packages/pandas/core/indexes/multi.py:609, in MultiIndex.from_tuples(cls, tuples, sortorder, names)
    607 if len(tuples) == 0:
    608     if names is None:
--> 609         raise TypeError("Cannot infer number of levels from empty list")
    610     # error: Argument 1 to "len" has incompatible type "Hashable";
    611     # expected "Sized"
    612     arrays = [[]] * len(names)  # type: ignore[arg-type]
TypeError: Cannot infer number of levels from empty list

Probably because of the same underlying issue, MultiIndex.factorize always loses the names of the original index. It should preserve the original names instead.

Expected Behavior

First factorize() should return (np.array([], dtype=np.intp), empty_ix)

Second example should return ['a', 'i'] instead of [None, None]

Installed Versions

INSTALLED VERSIONS ------------------ commit : fd3f57170aa1af588ba877e8e28c158a20a4886d python : 3.11.6.final.0 python-bits : 64 OS : Linux OS-release : 4.18.0-348.20.1.el8_5.x86_64 Version : #1 SMP Thu Mar 10 20:59:28 UTC 2022 machine : x86_64 processor : x86_64 byteorder : little LC_ALL : None LANG : en_GB.UTF-8 LOCALE : en_GB.UTF-8 pandas : 2.2.0 numpy : 1.26.3 pytz : 2023.3.post1 dateutil : 2.8.2 setuptools : 69.0.3 pip : 23.3.2 Cython : None pytest : 7.4.4 hypothesis : None sphinx : None blosc : None feather : None xlsxwriter : 3.1.9 lxml.etree : 5.1.0 html5lib : 1.1 pymysql : None psycopg2 : None jinja2 : 3.1.3 IPython : 8.20.0 pandas_datareader : None adbc-driver-postgresql: None adbc-driver-sqlite : None bs4 : 4.12.3 bottleneck : 1.3.7 dataframe-api-compat : None fastparquet : None fsspec : 2023.12.2 gcsfs : None matplotlib : 3.8.2 numba : 0.58.1 numexpr : None odfpy : None openpyxl : 3.1.2 pandas_gbq : None pyarrow : 14.0.2 pyreadstat : None python-calamine : None pyxlsb : None s3fs : None scipy : 1.12.0 sqlalchemy : None tables : None tabulate : None xarray : 2024.1.0 xlrd : 2.0.1 zstandard : None tzdata : 2023.4 qtpy : 2.4.1 pyqt5 : None
rhshadrach commented 8 months ago

Thanks for the report. Agreed this shouldn't raise, but I'm not certain about preserving names. pd.Index([2, 4, 3], name="a").factorize() also does not preserve the name, and doing so might have some wide ranging complications (haven't checked). Further investigations welcome - in particular, if we do preserve names, what is the impact on the test suite?

batterseapower commented 8 months ago

I also think names should be preserved in the regular Index case. It does break backwards compatibility to do this because existing code may be relying on them not being preserved, but leaving this aside it does seem very clear to me that dropping the names is surprising behaviour.

rhshadrach commented 8 months ago

but leaving this aside it does seem very clear to me that dropping the names is surprising behaviour.

No disagreement here offhand, but this could have wide ranging implications and the impact needs to be investigated.