BUG/API: DTI/TDI/PI.insert cast to object on failure
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry

ATM DTI/TDI/PI/insert are pretty idiosyncratic.  This changes them to match the base Index.insert by a) always trying _validate_fill_value (this changes the behavior for strings that can be parsed to datetime/timedelta/period) and b) always fall back to object in cases where this validation fails (this changes behavior for non-strings where the validation fails).

AFAICT the current behavior was implemented in #5819 and the only discussion I see about castable strings or non-castable non-strings is this [comment](https://github.com/pandas-dev/pandas/pull/5819#issuecomment-31454545), which I don't think is relevant any longer.

One corner case is mismatched tzs, which raise in master.  In the PR they cast to object.  They could also reasonably cast to the index's tz (xref #37605)

cc @rhshadrach IIRC you had a question about the insert tests in test_coercion.  I think this addresses what you were asking about.  Let me know if I missed something.

Labels:
name: Datetime
description: Datetime data dtype
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
xref #37774
> a more general question here. why are we trying to accomodate this at all with a non-conforming value? shouldn't we just plain raise? and leave it to the caller to astype(object)?

if we were starting from scratch that'd be a reasonable option, but implementing that consistently would break a lot of existing code.  The best we can do is try to be internally consistent.
> a more general question here. why are we trying to accomodate this at all with a non-conforming value? shouldn't we just plain raise? and leave it to the caller to astype(object)?

Agreed with this, and that is also why I have been arguing for this behaviour for the new extension dtypes. 

I certainly follow the goal to be consistent internally, but *if* we prefer the strict behavior, and *if* we do that for the new nullable extension dtypes (both "if"s to discuss), we will have some inconsistency for a period of time anyway. In which case it might make sense to *not* fix the remaining current inconsistencies as done here.
> why are we trying to accomodate this at all with a non-conforming value? shouldn't we just plain raise? and leave it to the caller to astype(object)?

This comes up in at least two fairly-common contexts:

1) `df[new_col] = foo` when `new_col` doesn't match `df.columns.dtype`
2) `obj.reset_index()`

It seems awkward to ask users to do `df.columns = df.columns.astype(object)` before that kind of operation, especially since the reset_index can happen somewhat deep inside other functions.

> and if we do that for the new nullable extension dtypes (both "if"s to discuss), we will have some inconsistency for a period of time anyway

I'm not sure i follow.  This is about `Index.insert`, so nullable dtypes are still aspirational.
Ah yes, sorry, I had "arrays" in my head when writing that, and not "indexes" (we still need to have the discussion for arrays, will need to open a dedicated issue about that at some point). 

Fully agreed that those use cases you mention are important for Index objects where automatic upcasting is useful.

> AFAICT the current behavior was implemented in #5819 and the only discussion I see about castable strings or non-castable non-strings is this comment, which I don't think is relevant any longer.

No longer relevant in the sense that we no longer check if a string is datetime-like ? Or ..?

> One corner case is mismatched tzs, which raise in master. In the PR they cast to object. They could also reasonably cast to the index's tz (xref #37605)

Yes, and similarly as being discussed on that issue, my preference would be to not cast to object for this case, but preserve the original tz
i think the earlier non-consensus was a misunderstanding and outstanding issues have been resolved?
whatsnew added + green
thanks @jbrockmendel