-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
REF: use check_setitem_lengths in DTA.__setitem__ #36339
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
Changes from 4 commits
fba224d
4bf12b1
cb86e9e
e6e5e48
8bcd902
a8c0494
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 |
---|---|---|
|
@@ -114,7 +114,7 @@ def is_empty_indexer(indexer, arr_value: np.ndarray) -> bool: | |
# Indexer Validation | ||
|
||
|
||
def check_setitem_lengths(indexer, value, values) -> None: | ||
def check_setitem_lengths(indexer, value, values) -> bool: | ||
""" | ||
Validate that value and indexer are the same length. | ||
|
||
|
@@ -133,34 +133,48 @@ def check_setitem_lengths(indexer, value, values) -> None: | |
|
||
Returns | ||
------- | ||
None | ||
bool | ||
Whether this is an empty listlike setting which is a no-op. | ||
|
||
Raises | ||
------ | ||
ValueError | ||
When the indexer is an ndarray or list and the lengths don't match. | ||
""" | ||
# boolean with truth values == len of the value is ok too | ||
no_op = False | ||
|
||
if isinstance(indexer, (np.ndarray, list)): | ||
if is_list_like(value) and len(indexer) != len(value): | ||
if not ( | ||
isinstance(indexer, np.ndarray) | ||
and indexer.dtype == np.bool_ | ||
and len(indexer[indexer]) == len(value) | ||
): | ||
raise ValueError( | ||
"cannot set using a list-like indexer " | ||
"with a different length than the value" | ||
) | ||
# We can ignore other listlikes becasue they are either | ||
# a) not necessarily 1-D indexers, e.g. tuple | ||
# b) boolean indexers e.g. BoolArray | ||
if is_list_like(value): | ||
if len(indexer) != len(value): | ||
if not ( | ||
isinstance(indexer, np.ndarray) | ||
and indexer.dtype == np.bool_ | ||
and len(indexer[indexer]) == len(value) | ||
): | ||
# boolean with truth values == len of the value is ok too | ||
raise ValueError( | ||
"cannot set using a list-like indexer " | ||
"with a different length than the value" | ||
) | ||
if len(indexer) == 0: | ||
no_op = True | ||
|
||
elif isinstance(indexer, slice): | ||
# slice | ||
if is_list_like(value) and len(values): | ||
if len(value) != length_of_indexer(indexer, values): | ||
raise ValueError( | ||
"cannot set using a slice indexer with a " | ||
"different length than the value" | ||
) | ||
if is_list_like(value): | ||
if len(values): | ||
if len(value) != length_of_indexer(indexer, values): | ||
raise ValueError( | ||
"cannot set using a slice indexer with a " | ||
"different length than the value" | ||
) | ||
else: | ||
# TODO: dont we still need lengths to match? | ||
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 yes? 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. the existing code does not check for length-match in the len==0 case. ill take a look at adding that check and getting rid of this comment 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. It actually is checked right now, because of the As example:
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. Good catch; i was comparing against the existing |
||
no_op = True | ||
return no_op | ||
|
||
|
||
def validate_indices(indices: np.ndarray, n: int) -> None: | ||
|
Uh oh!
There was an error while loading. Please reload this page.