-
-
Notifications
You must be signed in to change notification settings - Fork 19.2k
CLN: Add allow_slice to is_hashable function #62567
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?
Changes from 30 commits
9a1c452
ee2bf48
70a8357
bae4e48
c324360
37fec55
859f2f8
90cb69e
4544845
0e4d675
03c1b22
8ded755
067b830
8fcecee
871d621
d145d0b
2b0ba79
2a6e7a1
7b83c16
0a57d73
c684c43
06af37c
fb3e85f
272a17b
4440d70
fd30506
62793a4
2a3a0a8
1331063
095f473
0c97a82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -793,8 +793,7 @@ def _get_setitem_indexer(self, key): | |
| if ( | ||
| isinstance(ax, MultiIndex) | ||
| and self.name != "iloc" | ||
| and is_hashable(key) | ||
| and not isinstance(key, slice) | ||
| and is_hashable(key, allow_slice=False) | ||
| ): | ||
| with suppress(KeyError, InvalidIndexError): | ||
| # TypeError e.g. passed a bool | ||
|
|
@@ -1147,8 +1146,7 @@ def _contains_slice(x: object) -> bool: | |
| # This should never be reached, but let's be explicit about it | ||
| raise ValueError("Too many indices") # pragma: no cover | ||
| if all( | ||
| (is_hashable(x) and not _contains_slice(x)) or com.is_null_slice(x) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this is the only usage of _contains_slice, so that can be removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @LirongMa can you address this |
||
| for x in tup | ||
| is_hashable(x, allow_slice=False) or com.is_null_slice(x) for x in tup | ||
| ): | ||
| # GH#10521 Series should reduce MultiIndex dimensions instead of | ||
| # DataFrame, IndexingError is not raised when slice(None,None,None) | ||
|
|
@@ -1511,12 +1509,8 @@ def _convert_to_indexer(self, key, axis: AxisInt): | |
|
|
||
| # Slices are not valid keys passed in by the user, | ||
| # even though they are hashable in Python 3.12 | ||
| contains_slice = False | ||
| if isinstance(key, tuple): | ||
| contains_slice = any(isinstance(v, slice) for v in key) | ||
|
|
||
| if is_scalar(key) or ( | ||
| isinstance(labels, MultiIndex) and is_hashable(key) and not contains_slice | ||
| isinstance(labels, MultiIndex) and is_hashable(key, allow_slice=False) | ||
|
||
| ): | ||
| # Otherwise get_loc will raise InvalidIndexError | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ | |
| missing as libmissing, | ||
| ops as libops, | ||
| ) | ||
| from pandas.compat import PY312 | ||
| from pandas.compat.numpy import np_version_gt2 | ||
| from pandas.errors import Pandas4Warning | ||
|
|
||
|
|
@@ -452,16 +453,55 @@ class UnhashableClass2: | |
| def __hash__(self): | ||
| raise TypeError("Not hashable") | ||
|
|
||
| class HashableSlice: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comment that we can get rid of that once we drop py311? |
||
| def __init__(self, start, stop, step=None): | ||
| self.slice = slice(start, stop, step) | ||
|
|
||
| def __eq__(self, other): | ||
| return isinstance(other, HashableSlice) and self.slice == other.slice | ||
|
|
||
| def __hash__(self): | ||
| return hash((self.slice.start, self.slice.stop, self.slice.step)) | ||
|
|
||
| def __repr__(self): | ||
| return ( | ||
| f"HashableSlice({self.slice.start}, {self.slice.stop}, " | ||
| f"{self.slice.step})" | ||
| ) | ||
|
|
||
| hashable = (1, 3.14, np.float64(3.14), "a", (), (1,), HashableClass()) | ||
| not_hashable = ([], UnhashableClass1()) | ||
| abc_hashable_not_really_hashable = (([],), UnhashableClass2()) | ||
| hashable_slice = HashableSlice(1, 2) | ||
| tuple_with_slice = (slice(1, 2), 3) | ||
|
|
||
| for i in hashable: | ||
| assert inference.is_hashable(i) | ||
| assert inference.is_hashable(i, allow_slice=True) | ||
| assert inference.is_hashable(i, allow_slice=False) | ||
| for i in not_hashable: | ||
| assert not inference.is_hashable(i) | ||
| assert not inference.is_hashable(i, allow_slice=True) | ||
| assert not inference.is_hashable(i, allow_slice=False) | ||
| for i in abc_hashable_not_really_hashable: | ||
| assert not inference.is_hashable(i) | ||
| assert not inference.is_hashable(i, allow_slice=True) | ||
| assert not inference.is_hashable(i, allow_slice=False) | ||
|
|
||
| assert inference.is_hashable(hashable_slice) | ||
| assert inference.is_hashable(hashable_slice, allow_slice=True) | ||
| assert inference.is_hashable(hashable_slice, allow_slice=False) | ||
|
|
||
| if PY312: | ||
| for obj in [slice(1, 2), tuple_with_slice]: | ||
| assert inference.is_hashable(obj) | ||
| assert inference.is_hashable(obj, allow_slice=True) | ||
| assert not inference.is_hashable(obj, allow_slice=False) | ||
| else: | ||
| for obj in [slice(1, 2), tuple_with_slice]: | ||
| assert not inference.is_hashable(obj) | ||
| assert not inference.is_hashable(obj, allow_slice=True) | ||
| assert not inference.is_hashable(obj, allow_slice=False) | ||
|
|
||
| # numpy.array is no longer collections.abc.Hashable as of | ||
| # https://github.com/numpy/numpy/pull/5326, just test | ||
|
|
||
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.
i think this extra comment is unnecessary given the description in the parameters section