-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
CI: Test Python 3.12 #53743
CI: Test Python 3.12 #53743
Changes from all commits
ff98491
fcbe1ed
dd4a488
c809bcd
cf7e272
8c7aea2
dc88fa8
d2b3868
5814bcc
7be7ea9
e9c0ed4
f735b8c
80d31e1
b8f351d
7cbfd7c
e507d45
90dbdeb
5e421f3
0e04fd2
b5ae510
b1182ac
ddae8f8
64e12a9
bc64b47
2c1755e
5dfc14b
a1bd210
298f31b
88a0cb8
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 |
---|---|---|
|
@@ -32,7 +32,10 @@ | |
to_offset, | ||
) | ||
from pandas.compat.numpy import function as nv | ||
from pandas.errors import NullFrequencyError | ||
from pandas.errors import ( | ||
InvalidIndexError, | ||
NullFrequencyError, | ||
) | ||
from pandas.util._decorators import ( | ||
Appender, | ||
cache_readonly, | ||
|
@@ -165,7 +168,7 @@ def __contains__(self, key: Any) -> bool: | |
hash(key) | ||
try: | ||
self.get_loc(key) | ||
except (KeyError, TypeError, ValueError): | ||
except (KeyError, TypeError, ValueError, InvalidIndexError): | ||
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. what cases get here? 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. |
||
return False | ||
return True | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1022,7 +1022,12 @@ def __getitem__(self, key): | |
elif key_is_scalar: | ||
return self._get_value(key) | ||
|
||
if is_hashable(key): | ||
# Convert generator to list before going through hashable part | ||
# (We will iterate through the generator there to check for slices) | ||
if is_iterator(key): | ||
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. why does changing the order matter? the current order is pretty fine-tuned for perf 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. We need to check for slices in iterators by iterating through them, since they aren't allowed as hash keys. IIUC, this should only lower perf for generators? |
||
key = list(key) | ||
|
||
if is_hashable(key) and not isinstance(key, slice): | ||
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. very late review comment: would it make sense to make is_hashable_non_slice or something? it wouldn't surprise me if many places that use is_hashable current assume non-slice but didn't get updated by this PR 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. Yeah sure, I just updated the places that broke in the tests. Is there a case where we would actually want 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 having is_hashable be anything other than a try/except around hash would cause problems. im suggesting a new function to de-duplicate the hashable-but-not-slice checks this introduces 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. Made an issue #55152. I don't have the same time that I had in summer to work on pandas, but I'll try to have a look (no promises, though). |
||
# Otherwise index.get_value will raise InvalidIndexError | ||
try: | ||
# For labels that don't resolve as scalars like tuples and frozensets | ||
|
@@ -1042,9 +1047,6 @@ def __getitem__(self, key): | |
# Do slice check before somewhat-costly is_bool_indexer | ||
return self._getitem_slice(key) | ||
|
||
if is_iterator(key): | ||
key = list(key) | ||
|
||
if com.is_bool_indexer(key): | ||
key = check_bool_indexer(self.index, key) | ||
key = np.asarray(key, dtype=bool) | ||
|
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.
the linked issue is now closed; can this be reenabled?
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.
Matt enabled this again a while back, I think.