Skip to content

Commit

Permalink
BUG: raise on non-hashable Index name, closes pandas-dev#29069 (panda…
Browse files Browse the repository at this point in the history
  • Loading branch information
jbrockmendel authored and AlexKirko committed Dec 29, 2019
1 parent 72bbc2e commit 77977b5
Show file tree
Hide file tree
Showing 13 changed files with 67 additions and 32 deletions.
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v1.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,7 @@ Other
- Fixed :class:`IntegerArray` returning ``inf`` rather than ``NaN`` for operations dividing by 0 (:issue:`27398`)
- Fixed ``pow`` operations for :class:`IntegerArray` when the other value is ``0`` or ``1`` (:issue:`29997`)
- Bug in :meth:`Series.count` raises if use_inf_as_na is enabled (:issue:`29478`)

- Bug in :class:`Index` where a non-hashable name could be set without raising ``TypeError`` (:issue:29069`)

.. _whatsnew_1000.contributors:

Expand Down
40 changes: 32 additions & 8 deletions pandas/core/indexes/base.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from datetime import datetime
import operator
from textwrap import dedent
from typing import FrozenSet, Union
from typing import FrozenSet, Hashable, Optional, Union
import warnings

import numpy as np
Expand Down Expand Up @@ -239,7 +239,7 @@ def _outer_indexer(self, left, right):
_typ = "index"
_data: Union[ExtensionArray, np.ndarray]
_id = None
name = None
_name: Optional[Hashable] = None
_comparables = ["name"]
_attributes = ["name"]
_is_numeric_dtype = False
Expand Down Expand Up @@ -274,8 +274,7 @@ def __new__(
from .interval import IntervalIndex
from .category import CategoricalIndex

if name is None and hasattr(data, "name"):
name = data.name
name = maybe_extract_name(name, data, cls)

if isinstance(data, ABCPandasArray):
# ensure users don't accidentally put a PandasArray in an index.
Expand Down Expand Up @@ -520,7 +519,7 @@ def _simple_new(cls, values, name=None, dtype=None):
# data buffers and strides. We don't re-use `_ndarray_values`, since
# we actually set this value too.
result._index_data = values
result.name = name
result._name = name

return result._reset_identity()

Expand Down Expand Up @@ -1209,6 +1208,15 @@ def to_frame(self, index=True, name=None):
# --------------------------------------------------------------------
# Name-Centric Methods

@property
def name(self):
return self._name

@name.setter
def name(self, value):
maybe_extract_name(value, None, type(self))
self._name = value

def _validate_names(self, name=None, names=None, deep=False):
"""
Handles the quirks of having a singular 'name' parameter for general
Expand Down Expand Up @@ -1258,7 +1266,7 @@ def _set_names(self, values, level=None):
for name in values:
if not is_hashable(name):
raise TypeError(f"{type(self).__name__}.name must be a hashable type")
self.name = values[0]
self._name = values[0]

names = property(fset=_set_names, fget=_get_names)

Expand Down Expand Up @@ -1546,7 +1554,7 @@ def droplevel(self, level=0):
if mask.any():
result = result.putmask(mask, np.nan)

result.name = new_names[0]
result._name = new_names[0]
return result
else:
from .multi import MultiIndex
Expand Down Expand Up @@ -1777,7 +1785,7 @@ def __setstate__(self, state):
nd_state, own_state = state
data = np.empty(nd_state[1], dtype=nd_state[2])
np.ndarray.__setstate__(data, nd_state)
self.name = own_state[0]
self._name = own_state[0]

else: # pragma: no cover
data = np.empty(state)
Expand Down Expand Up @@ -5462,3 +5470,19 @@ def default_index(n):
from pandas.core.indexes.range import RangeIndex

return RangeIndex(0, n, name=None)


def maybe_extract_name(name, obj, cls) -> Optional[Hashable]:
"""
If no name is passed, then extract it from data, validating hashability.
"""
if name is None and isinstance(obj, (Index, ABCSeries)):
# Note we don't just check for "name" attribute since that would
# pick up e.g. dtype.name
name = obj.name

# GH#29069
if not is_hashable(name):
raise TypeError(f"{cls.__name__}.name must be a hashable type")

return name
5 changes: 2 additions & 3 deletions pandas/core/indexes/category.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from pandas.core.base import _shared_docs
import pandas.core.common as com
import pandas.core.indexes.base as ibase
from pandas.core.indexes.base import Index, _index_shared_docs
from pandas.core.indexes.base import Index, _index_shared_docs, maybe_extract_name
import pandas.core.missing as missing
from pandas.core.ops import get_op_result_name

Expand Down Expand Up @@ -175,8 +175,7 @@ def __new__(

dtype = CategoricalDtype._from_values_or_dtype(data, categories, ordered, dtype)

if name is None and hasattr(data, "name"):
name = data.name
name = maybe_extract_name(name, data, cls)

if not is_categorical_dtype(data):
# don't allow scalars
Expand Down
1 change: 0 additions & 1 deletion pandas/core/indexes/datetimelike.py
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,6 @@ class DatetimelikeDelegateMixin(PandasDelegate):
_raw_methods: Set[str] = set()
# raw_properties : dispatch properties that shouldn't be boxed in an Index
_raw_properties: Set[str] = set()
name = None
_data: ExtensionArray

@property
Expand Down
5 changes: 2 additions & 3 deletions pandas/core/indexes/datetimes.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
)
from pandas.core.base import _shared_docs
import pandas.core.common as com
from pandas.core.indexes.base import Index
from pandas.core.indexes.base import Index, maybe_extract_name
from pandas.core.indexes.datetimelike import (
DatetimeIndexOpsMixin,
DatetimelikeDelegateMixin,
Expand Down Expand Up @@ -257,8 +257,7 @@ def __new__(

# - Cases checked above all return/raise before reaching here - #

if name is None and hasattr(data, "name"):
name = data.name
name = maybe_extract_name(name, data, cls)

dtarr = DatetimeArray._from_sequence(
data,
Expand Down
4 changes: 2 additions & 2 deletions pandas/core/indexes/interval.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
_index_shared_docs,
default_pprint,
ensure_index,
maybe_extract_name,
)
from pandas.core.indexes.datetimes import DatetimeIndex, date_range
from pandas.core.indexes.multi import MultiIndex
Expand Down Expand Up @@ -217,8 +218,7 @@ def __new__(
verify_integrity: bool = True,
):

if name is None and hasattr(data, "name"):
name = data.name
name = maybe_extract_name(name, data, cls)

with rewrite_exception("IntervalArray", cls.__name__):
array = IntervalArray(
Expand Down
10 changes: 7 additions & 3 deletions pandas/core/indexes/numeric.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@

from pandas.core import algorithms
import pandas.core.common as com
from pandas.core.indexes.base import Index, InvalidIndexError, _index_shared_docs
from pandas.core.indexes.base import (
Index,
InvalidIndexError,
_index_shared_docs,
maybe_extract_name,
)
from pandas.core.ops import get_op_result_name

_num_index_shared_docs = dict()
Expand Down Expand Up @@ -68,8 +73,7 @@ def __new__(cls, data=None, dtype=None, copy=False, name=None):
else:
subarr = data

if name is None and hasattr(data, "name"):
name = data.name
name = maybe_extract_name(name, data, cls)
return cls._simple_new(subarr, name=name)

@classmethod
Expand Down
9 changes: 6 additions & 3 deletions pandas/core/indexes/period.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@
from pandas.core.base import _shared_docs
import pandas.core.common as com
import pandas.core.indexes.base as ibase
from pandas.core.indexes.base import _index_shared_docs, ensure_index
from pandas.core.indexes.base import (
_index_shared_docs,
ensure_index,
maybe_extract_name,
)
from pandas.core.indexes.datetimelike import (
DatetimeIndexOpsMixin,
DatetimelikeDelegateMixin,
Expand Down Expand Up @@ -184,8 +188,7 @@ def __new__(
argument = list(set(fields) - valid_field_set)[0]
raise TypeError(f"__new__() got an unexpected keyword argument {argument}")

if name is None and hasattr(data, "name"):
name = data.name
name = maybe_extract_name(name, data, cls)

if data is None and ordinal is None:
# range-based.
Expand Down
4 changes: 2 additions & 2 deletions pandas/core/indexes/range.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import pandas.core.common as com
from pandas.core.construction import extract_array
import pandas.core.indexes.base as ibase
from pandas.core.indexes.base import Index, _index_shared_docs
from pandas.core.indexes.base import Index, _index_shared_docs, maybe_extract_name
from pandas.core.indexes.numeric import Int64Index
from pandas.core.ops.common import unpack_zerodim_and_defer

Expand Down Expand Up @@ -85,10 +85,10 @@ def __new__(
):

cls._validate_dtype(dtype)
name = maybe_extract_name(name, start, cls)

# RangeIndex
if isinstance(start, RangeIndex):
name = start.name if name is None else name
start = start._range
return cls._simple_new(start, dtype=dtype, name=name)

Expand Down
5 changes: 3 additions & 2 deletions pandas/core/indexes/timedeltas.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from pandas.core.arrays.timedeltas import TimedeltaArray, _is_convertible_to_td
from pandas.core.base import _shared_docs
import pandas.core.common as com
from pandas.core.indexes.base import Index, _index_shared_docs
from pandas.core.indexes.base import Index, _index_shared_docs, maybe_extract_name
from pandas.core.indexes.datetimelike import (
DatetimeIndexOpsMixin,
DatetimelikeDelegateMixin,
Expand Down Expand Up @@ -168,6 +168,7 @@ def __new__(
copy=False,
name=None,
):
name = maybe_extract_name(name, data, cls)

if is_scalar(data):
raise TypeError(
Expand Down Expand Up @@ -215,7 +216,7 @@ def _simple_new(cls, values, name=None, freq=None, dtype=_TD_DTYPE):
tdarr = TimedeltaArray._simple_new(values._data, freq=freq)
result = object.__new__(cls)
result._data = tdarr
result.name = name
result._name = name
# For groupby perf. See note in indexes/base about _index_data
result._index_data = tdarr._data

Expand Down
6 changes: 2 additions & 4 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ def __init__(

else:

name = ibase.maybe_extract_name(name, data, type(self))

if is_empty_data(data) and dtype is None:
# gh-17261
warnings.warn(
Expand All @@ -219,8 +221,6 @@ def __init__(
"initializing a Series from a MultiIndex is not supported"
)
elif isinstance(data, Index):
if name is None:
name = data.name

if dtype is not None:
# astype copies
Expand All @@ -244,8 +244,6 @@ def __init__(
)
pass
elif isinstance(data, ABCSeries):
if name is None:
name = data.name
if index is None:
index = data.index
else:
Expand Down
7 changes: 7 additions & 0 deletions pandas/tests/indexes/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ def test_shift(self):
with pytest.raises(NotImplementedError, match=msg):
idx.shift(1, 2)

def test_constructor_name_unhashable(self):
# GH#29069 check that name is hashable
# See also same-named test in tests.series.test_constructors
idx = self.create_index()
with pytest.raises(TypeError, match="Index.name must be a hashable type"):
type(idx)(idx, name=[])

def test_create_index_existing_name(self):

# GH11193, when an existing index is passed, and a new name is not
Expand Down
1 change: 1 addition & 0 deletions pandas/tests/indexes/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ def test_constructor_copy(self, index):
arr[0] = "SOMEBIGLONGSTRING"
assert new_index[0] != "SOMEBIGLONGSTRING"

# FIXME: dont leave commented-out
# what to do here?
# arr = np.array(5.)
# pytest.raises(Exception, arr.view, Index)
Expand Down

0 comments on commit 77977b5

Please sign in to comment.