Datetimelike __setitem__ #24477
Datetimelike __setitem__ #24477
Conversation
Split from pandas-dev#24024.
TomAugspurger
added
Datetime
| if is_list_like(value): | ||
| is_slice = isinstance(key, slice) | ||
| if (not is_slice | ||
| and len(key) != len(value) |
There was a problem hiding this comment.
key could be a scalar here right? (in which case u will get an odd exception about len of unsized object)
There was a problem hiding this comment.
Hrm... yeah
In [3]: pd.period_range('2000', periods=2)._data
Out[3]:
<PeriodArray>
['2000-01-01', '2000-01-02']
Length: 2, dtype: period[D]
In [4]: arr = pd.period_range('2000', periods=2)._data
In [5]: arr[0] = arr[[0, 1]]
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-5-9a9ffe90069d> in <module>
----> 1 arr[0] = arr[[0, 1]]
~/sandbox/pandas/pandas/core/arrays/datetimelike.py in __setitem__(self, key, value)
494 is_slice = isinstance(key, slice)
495 if (not is_slice
--> 496 and len(key) != len(value)
497 and not com.is_bool_indexer(key)):
498 msg = ("shape mismatch: value array of length '{}' does not "
TypeError: object of type 'int' has no len()This is a gap in the base extension tests as well. I'll ad one. that should be a... what? ValueError for trying to set list-like into a single element?
There was a problem hiding this comment.
NumPy raises with ValueError: setting an array element with a sequence. which seems like a fine error message to me.
There was a problem hiding this comment.
yep seems good
maybe just let it fall thru this if will work?
There was a problem hiding this comment.
Seemed a tad easier to explicitly check for a scalar key here.
Actually, grr, this is kinda annoying but NumPy allows setting a sequence of length 1.
In [2]: x = np.array([1, 2])
In [3]: x[0] = x[[0]]but now I would raise on that.
In [10]: arr = pd.period_range('2000', periods=2)._data
In [11]: arr[0] = arr[[0]]
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
<ipython-input-11-666653b9c59a> in <module>
----> 1 arr[0] = arr[[0]]
~/sandbox/pandas/pandas/core/arrays/datetimelike.py in __setitem__(self, key, value)
495
496 if lib.is_scalar(key):
--> 497 raise ValueError("setting an array element with a sequence.")
498
499 if (not is_slice
ValueError: setting an array element with a sequence.Thoughts? I think this should raise, since a length-1 sequence is more like a sequence than a scalar.
There was a problem hiding this comment.
yeah i think we raise on this
this is why setitem in Block is so complicated :)
Codecov Report
@@ Coverage Diff @@
## master #24477 +/- ##
===========================================
- Coverage 92.32% 43.06% -49.27%
===========================================
Files 166 166
Lines 52298 52303 +5
===========================================
- Hits 48285 22524 -25761
- Misses 4013 29779 +25766
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #24477 +/- ##
==========================================
+ Coverage 92.32% 92.32% +<.01%
==========================================
Files 166 166
Lines 52298 52306 +8
==========================================
+ Hits 48285 48294 +9
+ Misses 4013 4012 -1
Continue to review full report at Codecov.
|
|
Tested and fixed in 2dc85b7.
…On Fri, Dec 28, 2018 at 8:49 PM Jeff Reback ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pandas/core/arrays/datetimelike.py
<#24477 (comment)>:
> + def __setitem__(
+ self,
+ key, # type: Union[int, Sequence[int], Sequence[bool], slice]
+ value, # type: Union[NaTType, Scalar, Sequence[Scalar]]
+ ):
+ # type: (...) -> None
+ # I'm fudging the types a bit here. The "Scalar" above really depends
+ # on type(self). For PeriodArray, it's Period (or stuff coercible
+ # to a period in from_sequence). For DatetimeArray, it's Timestamp...
+ # I don't know if mypy can do that, possibly with Generics.
+ # https://mypy.readthedocs.io/en/latest/generics.html
+
+ if is_list_like(value):
+ is_slice = isinstance(key, slice)
+ if (not is_slice
+ and len(key) != len(value)
key could be a scalar here right? (in which case u will get an odd
exception about len of unsized object)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#24477 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABQHIng3dwWdFOZL3dAEkKHtBC6vT-bNks5u9thUgaJpZM4ZkX7a>
.
|
|
All green. |
| @@ -108,6 +108,8 @@ def astype(self, dtype, copy=True): | |||
|
|
|||
| def __setitem__(self, key, value): | |||
| if pd.api.types.is_list_like(value): | |||
There was a problem hiding this comment.
should prob import these like we normally do (is_like_like) at the top, but no big deal
| ) | ||
| raise TypeError(msg.format(scalar=self._scalar_type.__name__, | ||
| typ=type(value).__name__)) | ||
| self._data[key] = value |
There was a problem hiding this comment.
would be nice to move smoe of this logic a big higher up to EA base if possible and/or make some helper methods to avoid code duplication of EA's implementating setitem, but for another time.
|
thanks! |
Split from #24024.
This is mostly just a move from PeriodArray to DatetimelikeArray, with period-specific things replaced with
_check_compatible_with,_unbox_scalar, etc.This is under-tested at the moment, just the basics are actually hit. But I think this is OK for two reasons
_check_compatible_with, freq invalidation, etc.).