BUG: no mixed naive/aware checks with mixed string/datetime

[jbrockmendel]

import pandas as pd

vals = ["2023-10-25 14:46:31", pd.Timestamp.now("US/Pacific")]

pd.to_datetime(vals)                              # <- object, with warning
pd.to_datetime(vals, format="%Y-%m-%d %H:%M:%S")  # <- object, with warning
pd.to_datetime(vals, format="mixed")              # <- US/Pacific


pd.to_datetime(vals[::-1])                              # <- US/Pacific
pd.to_datetime(vals[::-1], format="%Y-%m-%d %H:%M:%S")  # <- object, with warning
pd.to_datetime(vals[::-1], format="mixed")              # <- US/Pacific

In array_to_datetime and array_strptime we have a check for tznaive-vs-tzaware datetime objects. We also have checks for mixed offset strings. But we don't have any checks for mixes between the two types. In the example above we only get a warning in cases that go through array_strptime (the actual warning is issued in a post-processing step)

Adding checks for these mixes breaks 7 tests (or 6 depending on how we treat empty strings). Most of these are cases that currently warn that are caused to raise.

4 of the broken tests are test_to_datetime_mixed_datetime_and_string_with_format_mixed_offsets_utc_false which passes [Timestamp_or_pydatetime_naive, string_aware] and checks that it warns. Adding the missing check causes it to raise instead of warn. Another failure in test_mixed_offsets_with_native_datetime_raises is effectively the same thing (expects a warning, gets an exception)

Related

• The fact that we get different behavior in the example above i think comes from using tslib.first_non_null as part of deciding whether or not to use array_strptime. I expect that is also part of the reversing behavior in BUG: to_datetime with mixed iso-str and ints #50721.
• When we get a tzaware datetime and a string-with-offset, we currently ignore the datetime object's tz and return the string's tz for inferred_tz

[jbrockmendel]

cc @MarcoGorelli i think the current warning was your brainchild. would you be OK with having some of those cases be changed to raise as a bugfix?

[MarcoGorelli]

from chat just now - yes, sounds fine, mixing naive strings with tz-aware timestamps strikes me as 'playing with fire' anyway

[jbrockmendel]

Addressing this is proving a hassle bc there are two different places where we can do checks and raise/warn. ATM with datetime-vs-datetime cases we check inside the loop, which a) means any exception message has the "at position i" bit, b) can have handling specified by the errors keyword and c) has a perf penalty by virtue of being inside a loop. With string-vs-string cases we check outside the loop, which is the opposite on all three counts.

Re-using the existing state variables found_tz/found_naive and checking for mismatch at the end is simple code-wise but leads to really ugly order-dependence of what kind of behavior you get.

I guess once the deprecation in #54014 is enforced we can use a uglier more-state-variables approach. Longer term we can simplify things. I'd prefer for that simplification to do the check once outside the loop, will need to do some profiling to determine how strong that preference is.