Fix/empty string datetimelike conversion/issue 36550

[nrebena]

• xref BUG: Categorical series .isin({pd.NaT, ...}) gives all-False #36550
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

Empty string conversion

The actual comportment of Timedelta, Timestamp and to_datetime is to return NaT.

import pandas as pd
pd.to_datetime("", errors="raise")
# NaT
pd.to_datetime("", errors="coerce")
# NaT
pd.to_datetime("", errors="ignore")
# NaT
pd.Timedelta("")
# NaT
pd.Timestamp("")
# NaT

I would propose to raise a ValueError instead, same as for string that are not datetimelike, as follow:

import pandas as pd
import pytest

with pytest.raises(ValueError):
   pd.to_datetime("", errors="raise")
pd.to_datetime("", errors="coerce")
# NaT
pd.to_datetime("", errors="ignore")
# ''

with pytest.raises(ValueError):
   pd.Timedelta("")
with pytest.raises(ValueError):
   pd.Timestamp("")

Tests

I added the relevant test for the expected comportment.
I still will have to fix some test that rely on the current behaviors.

Benchmark

I ran the following command
asv continuous -E virtualenv -f 1.1 upstream/master HEAD -b "^tslibs.(timestamp|timedelta|tslib)"

       before           after         ratio
     [8f6ec1e8]       [4393cc9e]
     <master>         <fix/empty-string-datetimelike-conversion/issue-36550>
+         173±1ns         202±10ns     1.17  tslibs.timestamp.TimestampProperties.time_is_leap_year(tzlocal(), 'B')
+         162±2ns          188±4ns     1.16  tslibs.timestamp.TimestampProperties.time_is_quarter_start(tzlocal(), None)
+         178±1ns         205±20ns     1.15  tslibs.timestamp.TimestampProperties.time_dayofweek(tzfile('/usr/share/zoneinfo/US/Central'), 'B')
+         163±2ns         186±10ns     1.15  tslibs.timestamp.TimestampProperties.time_is_year_end(tzlocal(), None)
+         162±1ns         185±10ns     1.15  tslibs.timestamp.TimestampProperties.time_is_year_end(tzfile('/usr/share/zoneinfo/US/Central'), None)
+         162±2ns         185±10ns     1.14  tslibs.timestamp.TimestampProperties.time_is_year_end(<UTC>, None)
+         169±2ns         193±10ns     1.14  tslibs.timestamp.TimestampProperties.time_is_quarter_end(tzlocal(), None)
+         164±2ns          186±4ns     1.14  tslibs.timestamp.TimestampProperties.time_is_quarter_start(tzutc(), None)
+     4.60±0.03μs       5.16±0.3μs     1.12  tslibs.timestamp.TimestampProperties.time_is_year_end(datetime.timezone(datetime.timedelta(seconds=3600)), 'B')
+         147±1ns          163±4ns     1.11  tslibs.timestamp.TimestampProperties.time_microsecond(<DstTzInfo 'Europe/Amsterdam' LMT+0:20:00 STD>, 'B')
+         170±2ns          188±7ns     1.11  tslibs.timestamp.TimestampProperties.time_is_quarter_end(tzfile('/usr/share/zoneinfo/US/Central'), None)
+       179±0.7ns         197±10ns     1.10  tslibs.timestamp.TimestampProperties.time_dayofweek(datetime.timezone(datetime.timedelta(seconds=3600)), None)
+       174±0.3ns         191±10ns     1.10  tslibs.timestamp.TimestampProperties.time_is_leap_year(<DstTzInfo 'Europe/Amsterdam' LMT+0:20:00 STD>, 'B')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.

[jreback]

why this change? likely to break test parsing generally and would have to see a lot of benefit

[nrebena]

Considering empty string equivalent to NaT as a huge influence on all method that try to cast data into datetimelike (happen in groupby, merge, join, equal, probably others methods)
Globaly, I think we should refrain from implicit conversion of string to datetimelike, and this PR goes one step in this direction.
An other issue related to mixing string and datetimelike can be found here #36545

For information the broken test were the following:

pandas/tests/io/parser/test_parse_dates.py::test_parse_dates_empty_string[c_high]
pandas/tests/io/parser/test_parse_dates.py::test_parse_dates_empty_string[c_low]
pandas/tests/io/parser/test_parse_dates.py::test_parse_dates_empty_string[python]
pandas/tests/plotting/test_datetimelike.py::TestTSPlot::test_ts_plot_with_tz['UTC']
pandas/tests/tools/test_to_datetime.py::TestToDatetimeMisc::test_to_datetime_types[True]
pandas/tests/tools/test_to_datetime.py::TestToDatetimeMisc::test_to_datetime_types[False]
pandas/tests/tools/test_to_timedelta.py::TestTimedeltas::test_to_timedelta

If empty string should be always considered as NaT, then I'll close this PR.

[jreback]

does this affect read_csv? do we have tests for this?

this needs a fairly big section i the whatsnew as this can break existing code

[jreback]

I would instead remove the empty and put this in a new test for that case (and assert the error)

these are duplicating the constructor tests so should have either / or (i think they are fine here)

[jreback]

does this actually close the referenced issue?

[jreback]

can you merge master

[arw2019]

Closing for now. @nrebena let us know if you'd like to start this up again and we'll reopen