Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: raise on non-hashable Index name, closes #29069 #30335

Merged
merged 10 commits into from
Dec 27, 2019

Conversation

jbrockmendel
Copy link
Member

@jbrockmendel jbrockmendel commented Dec 18, 2019

@TomAugspurger
Copy link
Contributor

To close #29069, Index.name should probably be a settable property, and the setter should validate that the name is hashable.

Any perf concerns here? Things should be OK right?

@jbrockmendel
Copy link
Member Author

huh, looks like i got confused by names, which is a property. will update.

pandas/core/indexes/base.py Outdated Show resolved Hide resolved
@jreback jreback added Bug Index Related to the Index class or subclasses labels Dec 20, 2019
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also add a whatsnew note

@jbrockmendel
Copy link
Member Author

Are we OK with being able to set MultiIndex.name? When I tried disallowing it, some tests broke

@jbrockmendel
Copy link
Member Author

xref #11979 should we try to address that here while were at it?

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. some changes


# GH#29069
if not is_hashable(name):
raise TypeError(f"{cls_name}.name must be a hashable type")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you use type(obj).name here?

@@ -5464,3 +5472,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_name="Index"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you type and add a doc-string

@jreback jreback added this to the 1.0 milestone Dec 24, 2019
@jbrockmendel
Copy link
Member Author

I think #19171 becomes easier after this.


@name.setter
def name(self, value):
maybe_extract_name(value, None, type(self))
Copy link
Contributor

@jreback jreback Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in series.py you can change name.setter to use maybe_extract_name (ok for followon)

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments for a followon

@@ -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]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment above, can be more general so need to relocate i think as this is common between series/index)

@jreback jreback merged commit 698f689 into pandas-dev:master Dec 27, 2019
@jreback
Copy link
Contributor

jreback commented Dec 27, 2019

thanks

@jbrockmendel jbrockmendel deleted the bug-name-hash branch December 27, 2019 18:05
AlexKirko pushed a commit to AlexKirko/pandas that referenced this pull request Dec 29, 2019
keechongtan added a commit to keechongtan/pandas that referenced this pull request Dec 29, 2019
…ndexing-1row-df

* upstream/master: (333 commits)
  CI: troubleshoot Web_and_Docs failing (pandas-dev#30534)
  WARN: Ignore NumbaPerformanceWarning in test suite (pandas-dev#30525)
  DEPR: camelCase in offsets, get_offset (pandas-dev#30340)
  PERF: implement scalar ops blockwise (pandas-dev#29853)
  DEPR: Remove Series.compress (pandas-dev#30514)
  ENH: Add numba engine for rolling apply (pandas-dev#30151)
  [ENH] Add to_markdown method (pandas-dev#30350)
  DEPR: Deprecate pandas.np module (pandas-dev#30386)
  ENH: Add ignore_index for df.drop_duplicates (pandas-dev#30405)
  BUG: The setting xrot=0 in DataFrame.hist() doesn't work with by and subplots pandas-dev#30288 (pandas-dev#30491)
  CI: Fix GBQ Tests (pandas-dev#30478)
  Bug groupby quantile listlike q and int columns (pandas-dev#30485)
  ENH: Add ignore_index for df.sort_values and series.sort_values (pandas-dev#30402)
  TYP: Typing hints in pandas/io/formats/{css,csvs}.py (pandas-dev#30398)
  BUG: raise on non-hashable Index name, closes pandas-dev#29069 (pandas-dev#30335)
  Replace "foo!r" to "repr(foo)" syntax pandas-dev#29886 (pandas-dev#30502)
  BUG: preserve EA dtype in transpose (pandas-dev#30091)
  BLD: add check to prevent tempita name error, clsoes pandas-dev#28836 (pandas-dev#30498)
  REF/TST: method-specific files for test_append (pandas-dev#30503)
  marked unused parameters (pandas-dev#30504)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Index Related to the Index class or subclasses
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Index.name is not validatied to be Hashable
3 participants