-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
BUG: fix: list
as index item does not raise
#61674
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
base: main
Are you sure you want to change the base?
BUG: fix: list
as index item does not raise
#61674
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
# 60925 should raise when one of index's items is a list and others are not | ||
if any(isinstance(el, list) for el in data) and not all( | ||
isinstance(el, list) for el in data | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No non-hashable value should occur here. Instead, can you do this check on arr
below, and only in the case it's (a) a NumPy array (not an ExtensionArray) and (b) of object dtype. Use pandas.core.dtypes.inference.is_hashable
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for the review!
OK, will do all the changes.
Just didn't get what you mean by "do this check on arr below", do you mean in the try-except
block in line 570? Or inside the sanitize_array
definition? Or do the check in the arr
resulted from the sanitize_array
call?
Sorry for the confusion, newbie here, all those approaches seems OK to me, just asking which one you meant.
pandas/pandas/core/indexes/base.py
Lines 566 to 577 in 1da0d02
if len(data) and isinstance(data[0], tuple): | |
# Ensure we get 1-D array of tuples instead of 2D array. | |
data = com.asarray_tuplesafe(data, dtype=_dtype_obj) | |
try: | |
arr = sanitize_array(data, None, dtype=dtype, copy=copy) | |
except ValueError as err: | |
if "index must be specified when data is not list-like" in str(err): | |
raise cls._raise_scalar_data_error(data) from err | |
if "Data must be 1-dimensional" in str(err): | |
raise ValueError("Index data must be 1-dimensional") from err | |
raise |
@@ -71,9 +71,12 @@ def test_assign_index_sequences(self): | |||
repr(df) | |||
|
|||
# this travels an improper code path | |||
# 60925 should raise when one of index's items is a list and others are not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use GH#60925
throughout.
df.index = index | ||
repr(df) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it should raise on the first line, is that right? If so, can you remove repr(df)
.
@@ -749,6 +749,7 @@ Indexing | |||
- Bug in :meth:`MultiIndex.insert` when a new value inserted to a datetime-like level gets cast to ``NaT`` and fails indexing (:issue:`60388`) | |||
- Bug in printing :attr:`Index.names` and :attr:`MultiIndex.levels` would not escape single quotes (:issue:`60190`) | |||
- Bug in reindexing of :class:`DataFrame` with :class:`PeriodDtype` columns in case of consolidated block (:issue:`60980`, :issue:`60273`) | |||
- Bug in creating :class:`Index` with one item being a ``list`` among others that aren't (should raise ValueError) (:issue:`60925`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Bug in creating :class:`Index` with one item being a ``list`` among others that aren't (should raise ValueError) (:issue:`60925`) | |
- Bug in creating :class:`Index` with some items being a non-hashable; this now raises a ``ValueError`` (:issue:`60925`) |
Index.drop_duplicates()
is inconsistent for unhashable values #60925doc/source/whatsnew/v3.0.0.rst
file if fixing a bug or adding a new feature.Problem:
Index constructor was allowing creation of indexes where one of the index's item is a list (unhashable) while others are not lists.
Solution:
Added a check in the Index constructor for this case. Raise ValueError.
Added test to check if is raising correctly.
Changed a test that should raise.