BUG: Fix insertion of wrong-dtypes NaT into Series[m8ns] #27323
Conversation
There was a problem hiding this comment.
not sure I agree with the point of this PR. the np.* based 'NaT' should never exist in pandas at all, ever, we simply need to cast them appropriately; I get that you want to turn an M8 array when presented with an m8('NaT') into object, but I do't think we currently do. However I don't view this distinct as very import and it adds a lot of complexity. We now treat NaT, np.nan, None and np.*('nat') synonymously as missing values when presented to say a M8 (or a m8); I don't see why we don't simply say, hey I have a missig value, great use the value for this dtype.
What you are trying to do here IMHO is inserting even more of numpy issues into pandas when we are trying to do the opposite.
jreback
added
the
Missing-data
xref #24983. What you are suggesting is 100% impossible to do correctly.
Well you're right that it is not what we currently do, but that is a bug. It is unambiguously the correct behavior.
It's your prerogative (a word whose spelling never ceases to amaze me) to consider it a low-priority bug. On the flip side, I see complexity stemming from the fact that we have to guess what dtype NaT is behaving as in any given situation, and the impossibility of always guessing correctly.
Consider: if a user passes |
| elif util.is_datetime64_object(value): | ||
| # exclude np.datetime64("NaT") which would otherwise be picked up | ||
| # by the `value != value check below | ||
| pass |
There was a problem hiding this comment.
really everything from 531 down to 555 should be ripped out and this handled by the DatetimeArray/TimedeltaArray __setitem__ implementation
|
changed test class to just a function |
|
rebased following can_hold_element fix |
|
gentle ping; this one is a bugfix, admittedly a small one |
|
sure |
|
thanks @jbrockmendel |
Broken off from #27311 to troubleshoot build-specific failures. Also re-wrote test to parametrize and be more succinct