BUG: Fix inserting of wrong-dtyped NaT, closes #27297 #27311
Conversation
Might be because it addresses three separate-but-related bugs. Looking at it commit-by-commit might make more sense. I'm happy to make separate PRs if that's easier |
jreback
added
Missing-data
|
Rebased following #27323. This does for datetime64 Series what that did for timedelta64 Series |
|
no whatsnew, so doesnt need to be moved |
| @@ -2585,8 +2585,11 @@ def setitem(self, indexer, value): | |||
| try: | |||
| return super().setitem(indexer, value) | |||
| except (ValueError, TypeError): | |||
| obj_vals = self.values.astype(object) | |||
There was a problem hiding this comment.
do you really need to do this?
if you actually do, then .reshape(1, -1) should just work (or use np.atleast_2d)
There was a problem hiding this comment.
Yes, this really is needed.
reshape(1, -1) is what is already here
There was a problem hiding this comment.
can you give an example that specific needs this 2d reshape?
this just is a completely different pattern than anywhere else. Why for example would the ObjectBlock construct not simply reshape (if needed); it already knows the dim & the values shape?
| @@ -2585,8 +2585,11 @@ def setitem(self, indexer, value): | |||
| try: | |||
| return super().setitem(indexer, value) | |||
| except (ValueError, TypeError): | |||
| obj_vals = self.values.astype(object) | |||
There was a problem hiding this comment.
can you give an example that specific needs this 2d reshape?
this just is a completely different pattern than anywhere else. Why for example would the ObjectBlock construct not simply reshape (if needed); it already knows the dim & the values shape?
Because the Block constructor calls self._validate_ndim to check that it got the expected shape. reshaping is done earlier. |
is this not exactly what |
It is, but block_shape gets called before the constructor. But apparently it isnt necessary. Just ran the tests with this reshape disabled and its fine now. Will remove. |
|
removed+green |
| if not should_cast: | ||
| expected = expected.astype(object) | ||
|
|
||
| ser = base.copy(deep=True) |
There was a problem hiding this comment.
do these need to be deep? (no big deal though)
|
whatsnew added + green |
|
thanks @jbrockmendel |
|
It looks like this is causing a bunch of CI failures. cc @jreback @TomAugspurger |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffNeeds tests for PeriodDType case. The tests are probably not in the ideal place, how to move/parametrize them depends on what axis we want to sort them along (i.e. all Series indexing tests together or all timedelta-insertion tests together)