Regression in DataFrame.set_index with class instance column keys

[wkschwartz]

The following code worked in Pandas 0.23.4 but not in Pandas 0.24.0 (I'm on Python 3.7.2).

import pandas as pd

class Thing:
    # (Production code would also ensure a Thing instance's hash
    # and equality testing depended on name and color)

    def __init__(self, name, color):
        self.name = name
        self.color = color
    
    def __str__(self):
        return "<Thing %r>" % (self.name,)

thing1 = Thing('One', 'red')
thing2 = Thing('Two', 'blue')
df = pd.DataFrame({thing1: [0, 1], thing2: [2, 3]})
df.set_index([thing2])

In Pandas 0.23.4, I get the following correct result:

               <Thing 'One'>
<Thing 'Two'>               
2                          0
3                          1

In Pandas 0.24.0, I get the following error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ".../venv/lib/python3.7/site-packages/pandas/core/frame.py", line 4153, in set_index
    raise ValueError(err_msg)
ValueError: The parameter "keys" may be a column key, one-dimensional array, or a list containing only valid column keys and one-dimensional arrays.

After looking at Pandas 0.24.0's implementation of DataFrame.set_index:

pandas/pandas/core/frame.py

Lines 4144 to 4153 in 83eb242

	for col in keys:
	if (is_scalar(col) or isinstance(col, tuple)):
	# if col is a valid column key, everything is fine
	# tuples are always considered keys, never as list-likes
	if col not in self:
	missing.append(col)
	elif (not isinstance(col, (ABCIndexClass, ABCSeries,
	np.ndarray, list))
	or getattr(col, 'ndim', 1) > 1):
	raise ValueError(err_msg)

I noticed that is_scalar returns False for thing1 in Pandas 0.24.0:

>>> from pandas.core.dtypes.common import is_scalar
>>> is_scalar(thing1)
False

I suspect that it is incorrect to test DataFrame column keys using is_scalar.

Output of pd.show_versions()

pd.show_versions() from Pandas 0.23.4

INSTALLED VERSIONS

commit: None
python: 3.7.2.final.0
python-bits: 64
OS: Darwin
OS-release: 17.7.0
machine: x86_64
processor: i386
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8

pandas: 0.23.4
pytest: None
pip: 18.1
setuptools: 40.4.3
Cython: None
numpy: 1.16.0
scipy: 1.1.0
pyarrow: None
xarray: None
IPython: 7.2.0
sphinx: None
patsy: None
dateutil: 2.7.3
pytz: 2018.5
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: None
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: 1.1.2
lxml: None
bs4: None
html5lib: None
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None

pd.show_versions() from Pandas 0.24.0

INSTALLED VERSIONS

commit: None
python: 3.7.2.final.0
python-bits: 64
OS: Darwin
OS-release: 17.7.0
machine: x86_64
processor: i386
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8

pandas: 0.24.0
pytest: None
pip: 18.1
setuptools: 40.4.3
Cython: None
numpy: 1.16.0
scipy: 1.1.0
pyarrow: None
xarray: None
IPython: 7.2.0
sphinx: None
patsy: None
dateutil: 2.7.3
pytz: 2018.5
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: None
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: 1.1.2
lxml.etree: None
bs4: None
html5lib: None
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None
gcsfs: None

[jorisvandenbossche]

We have had quite some discussion lately about set_index (see eg #24046), and the actual change (that started to use is_scalar, I think) that caused this regression is #22486 and #24762

In general the usage is_scalar gives problems with custom objects. Eg we also fixed this in fillna (#20411).

cc @h-vetinari

[h-vetinari]

@jorisvandenbossche

Does pandas support custom objects as labels? I think that's bound to break in many places. The code previously tried everything it got as a key, so in this sense this is a regression, yes.

I'm a bit stumped as for how to deal with this. Column keys should IMO clearly be scalar (or tuples, grudgingly) - and that's the only reason is_scalar is there. CC @jreback

@wkschwartz

Your object (at least the toy version) looks a bit like a tuple. As an immediate workaround, I'd suggest to try inheriting Thing from tuple, then the isinstance(..., tuple)-side should work at least.

[wkschwartz]

In my production code, I use dataclasses as custom objects in both column keys and row indices, which worked throughout Pandas 0.23.4. If Pandas 0.24 or later drop support for custom classes in row/column indices, I would be stuck at 0.23.4 forever. This is why I view the change as a regression.

[TomAugspurger]

Does pandas support custom objects as labels?

We didn't disallow it previously, so yes.

[jreback]

This may have happened to work, but we don't support custom objects as labels explicity. Not against reverting this, but its buyer beware here.

[wkschwartz]

I never could find anything in the documentation that takes a stance on what can or can’t be column keys, except the general notion that DataFrames are dict-like. From this I surmised that column keys should be hashable and immutable. Did I miss something in the documentation?

[h-vetinari]

I've added a fix for the regression in #24984, with the caveat that I think that this should be immediately deprecated. For a method that's supposed to intelligently process column labels, arrays and list of labels/arrays, it's opening Pandora's box to try to support custom types. There's no end to the (hashable) horrors that someone might implement (see below).

Allowing dataclasses, as in the usecase of @wkschwartz is a separate consideration, but for much the same reasons, I'd suggest to deprecate even tuples in labels: #24688

See also #24702, which also ties into the following:

what can or can’t be column keys

This should be clearly defined, documented, and enforced (same as list-like).

---

About the stuff that pandas already "supports", how about lists-as-labels...?

>>> from pyfrozen import FrozenList
>>> class FroList(FrozenList):
...     def __init__(self, values):
...         super().__init__()
...         self.extend(values)
...         self.freeze()
...     def __hash__(self):
...         return hash(tuple(self))
...
>>> pd.DataFrame({FroList([1, 2, 3]): ['a', 'b', 'c']})
  [1, 2, 3]
0         a
1         b
2         c

[wkschwartz]

Obviously I would prefer no deprecation of custom label types as they are integral to my company’s applications. However, I would urge strongly that if you do decide to deprecate the feature, you do so starting only in the next major release (presumably 0.25.0) rather than in a minor release (0.24.1).

If you do stop supporting custom label types and I am not to be stuck at Pandas 0.23.4 forever, I could theoretically undertake the (expensive) refactoring to use unique IDs (my production code has the equivalent of the name field from my toy example in the OP). However, other users whose code would break might not have convenient unique IDs to switch to. Please do not remove this feature lightly.

[toobaz]

About the stuff that pandas already "supports", how about lists-as-labels...?

FroList is just a mistake, because it has a __hash__ which changes during its lifetime. If a user does stuff like this, he's looking for trouble: fair enough, not our fault.

[WillAyd]

Hmm well this conversation is somewhat split between this and the PR but I would be -1 on supporting something like this. I understand that it may have worked previously and wasn't explicitly disallowed, but I don't think that means we have to support it going forward, when I would think there are already an infinite other more idiomatic ways of going about this.

Even if fixed in this particular instance trying to guarantee support for something like this as an index item is going to cause a lot of unnecessary complexity in other parts of the code base, I would think particularly on subsequent indexing operations

[h-vetinari]

It does not change, that's what the .freeze is for...

It would work. Not that it should, of course, much less that it should be supported.

>>> ell = FroList([1, 2, 3])
>>> ell[1] = 'a'
Traceback (most recent call last):
[...]
RuntimeError: Cannot modify frozen list

[toobaz]

It does not change, that's what the .freeze is for...

I meant: it can contain a proper list. But you're right this is irrelevant (tuples can too). And indeed it works, and that's great. Users will need to remember only a simple rule: "hashable -> works". By the way: it's a rule they already know from dicts.