pandas-dev / pandas

Flexible and powerful data analysis / manipulation library for Python, providing labeled data structures similar to R data.frame objects, statistical functions, and much more
https://pandas.pydata.org
BSD 3-Clause "New" or "Revised" License
43.62k stars 17.91k forks source link

BUG: Change of behavior in casting of datetime-like types in MultiIndex #43091

Closed jgmarcel closed 2 years ago

jgmarcel commented 3 years ago

Code Sample, a copy-pastable example

[Edited to inform a much simpler example.]

import datetime
import pandas as pd

print(f"Pandas version:\t{pd.__version__}\n")

df = pd.DataFrame({'date': [datetime.date(2021, 8, 1),
                            datetime.date(2021, 8, 2),
                            datetime.date(2021, 8, 3)],
                   'ticker': ['aapl', 'goog', 'yhoo'],
                   'value': [5.63269, 4.45609, 2.74843]})

df.set_index(['date', 'ticker'], inplace=True)

print(df.index.get_level_values(0))

Output

The output below has been generated with pandas 1.3.0 or higher.

Pandas version: 1.3.0

Index([2021-08-01, 2021-08-02, 2021-08-03], dtype='object', name='date')

Expected Output

The output below has been generated with pandas 1.2.5.

Pandas version: 1.2.5

DatetimeIndex(['2021-08-01', '2021-08-02', '2021-08-03'], dtype='datetime64[ns]', name='date', freq=None)

Problem description

Starting from pandas 1.3.0, the observed behavior changed: in a MultiIndex creation, datetime.date objects are not cast to datetime64 anymore. I fail to find in the What’s new page the reason for that change of behavior. Is it by design or a bug?

Output of pd.show_versions()

``` INSTALLED VERSIONS ------------------ commit : 5f648bf1706dd75a9ca0d29f26eadfbb595fe52b python : 3.9.6.final.0 python-bits : 64 OS : Darwin OS-release : 19.6.0 Version : Darwin Kernel Version 19.6.0: Tue Jun 22 19:49:55 PDT 2021; root:xnu-6153.141.35~1/RELEASE_X86_64 machine : x86_64 processor : i386 byteorder : little LC_ALL : en_US.UTF-8 LANG : en_US.UTF-8 LOCALE : en_US.UTF-8 pandas : 1.3.2 numpy : 1.21.2 pytz : 2021.1 dateutil : 2.8.2 pip : 21.2.4 setuptools : 57.4.0 Cython : None pytest : None hypothesis : None sphinx : None blosc : None feather : None xlsxwriter : None lxml.etree : None html5lib : None pymysql : None psycopg2 : 2.9.1 (dt dec pq3 ext lo64) jinja2 : None IPython : 7.26.0 pandas_datareader: None bs4 : None bottleneck : None fsspec : None fastparquet : None gcsfs : None matplotlib : None numexpr : None odfpy : None openpyxl : None pandas_gbq : None pyarrow : None pyxlsb : None s3fs : None scipy : None sqlalchemy : 1.4.22 tables : None tabulate : None xarray : None xlrd : None xlwt : None numba : None ```
simonjayhawkins commented 3 years ago

Thanks @jgmarcel for the report.

first bad commit: [545a942424a26c4163e1f959ac6130984fc3fb41] BUG: Index([date]).astype("category").astype(object) roundtrip (#38552)

I'll mark as a regression for now pending further investigation.

Note that the set_index followed by a reset_index still creates a datetime64[ns] column from the original object column of date objects.

cc @jbrockmendel

jbrockmendel commented 3 years ago

Note that the set_index followed by a reset_index still creates a datetime64[ns] column from the original object column of date objects.

38552 was about round-tripping faithfully, so im inclined to think that reset_index should also roundtrip.

@jgmarcel support for date objects in generally is spotty. Your best bet is to use datetime or Timestamp objects

jgmarcel commented 3 years ago

@jgmarcel support for date objects in generally is spotty. Your best bet is to use datetime or Timestamp objects

Agreed. Thank you for the good advice. Problem is data objects are all over our code, since anytime a DATE column is read via pandas.read_sql(), it is translated to a series of date objects. Hence, casting everything to datetime or Timestamp objects would be a lot of effort… For now, we reverted to version 1.2.5, but I would like to avoid the work if possible.

jgmarcel commented 3 years ago

Hi guys,

Do you believe it will be possible to get this fixed on version 1.3.4? I ask so I can better plan the effort of adapting our whole code base.

Thank you.

jreback commented 3 years ago

@jgmarcel would likely take a community pull request

core can provide review

might be quite tricky as date have very little support

simonjayhawkins commented 3 years ago

changing milestone to 1.3.5

jreback commented 2 years ago

pls look at the 1.3.x release notes. IIRC this was changed on purpose in response to unforced conversion (e.g. to datetime times) it was desirable to keep datetime.date.

jgmarcel commented 2 years ago

Hi @jreback,

pls look at the 1.3.x release notes. IIRC this was changed on purpose in response to unforced conversion (e.g. to datetime times) it was desirable to keep datetime.date.

As initially stated, «I fail to find in the What’s new page the reason for that change of behavior». Would you mind pointing it out to me?

Anyway, the version policy clearly states that «API breaking changes should only occur in major releases», and «a deprecation path will be provided rather than an outright breaking change». Wouldn’t that be the case here, since we are talking about a breaking change that occurred between versions 1.2.5 and 1.3.0?

cc @simonjayhawkins

simonjayhawkins commented 2 years ago

in 1.2.5..

pd.Index(
    [
        datetime.date(2021, 8, 1),
        datetime.date(2021, 8, 2),
        datetime.date(2021, 8, 3),
    ]
)

gives

Index([2021-08-01, 2021-08-02, 2021-08-03], dtype='object')

and using the DataFrame from the OP

df.set_index(["date"]).index

gives

Index([2021-08-01, 2021-08-02, 2021-08-03], dtype='object', name='date')

whereas for a MultiIndex

arr = [
    datetime.date(2021, 8, 1),
    datetime.date(2021, 8, 2),
    datetime.date(2021, 8, 3),
]
pd.MultiIndex.from_arrays([arr, arr]).levels[0]

gives

DatetimeIndex(['2021-08-01', '2021-08-02', '2021-08-03'], dtype='datetime64[ns]', freq=None)

So the Index and MultiIndex constructors were inconsistent in the handling of object dtype arrays containing datetime objects in pandas 1.2.5.

As initially stated, «I fail to find in the What’s new page the reason for that change of behavior». Would you mind pointing it out to me?

The change of behavior in casting of datetime-like types in MultiIndex was done in #38552. Looking at the code changes in that PR, it is clear from the changed tests and comments added that this change was intentional. Unfortunately the release note added did not refer to changes in MultiIndex construction.

Anyway, the version policy clearly states that «API breaking changes should only occur in major releases», and «a deprecation path will be provided rather than an outright breaking change». Wouldn’t that be the case here, since we are talking about a breaking change that occurred between versions 1.2.5 and 1.3.0?

The policy also states

pandas will sometimes make behavior changing bug fixes, as part of minor or patch releases. Whether or not a change is a bug fix or an API-breaking change is a judgement call. We’ll do our best, and we invite you to participate in development discussion on the issue tracker or mailing list.

So the change in behavior could be considered a bug fix, since the MultiIndex constructor was inconsistent with the Index constructor and no further action.

However, the policy also states

Whenever possible, a deprecation path will be provided rather than an outright breaking change.

and

We will not introduce new deprecations in patch releases.

So, as an alternative, we could maybe restore the old behavior for 1.3.5 and add a deprecation of this behavior in 1.4

The only code change in #38552 was removing convert_dates=True from values = maybe_infer_to_datetimelike(values, convert_dates=True)

I guess we could maybe pass a convert_dates parameter through to the Categorical constructor from the MultiIndex constructor. @jbrockmendel wdyt?

jreback commented 2 years ago

-1 on any change here we have very limited if any support for datetime.date

not adding more complexity

jgmarcel commented 2 years ago

Hi @simonjayhawkins,

Your thorough explanation is very much appreciated. I see now how the change in behavior was more of a bug fix than an API-breaking change.

If I may pick your brain here, what do you believe would be the best way to achieve the pre-1.3 behavior, i.e. having datetime.date objects cast to datetime64 in MultiIndex? Would it be to call pd.to_datetime(arg, errors='ignore') where arg takes every column of the DataFrame (since I do not know in advance what its dtypes are)? Would it be saner to do that conversion immediately before or immediately after calling the MultiIndex constructor? Any other solution?

Thank you.

jbrockmendel commented 2 years ago

I guess we could maybe pass a convert_dates parameter through to the Categorical constructor from the MultiIndex constructor. @jbrockmendel wdyt?

It's possible. Though we'd then have a breaking change for anyone relying on the 1.3 behavior.

Would it be to call pd.to_datetime(arg, errors='ignore') where arg takes every column of the DataFrame

I'd check Index(col).inferred_type == "date"

jgmarcel commented 2 years ago

I'd check Index(col).inferred_type == "date"

That would be a great addition, yes. Thank you for that! However, I would also have to check for an inferred type of mixed, for when my column of datetime.date objects contains null dates, right?

simonjayhawkins commented 2 years ago

removing this issue from the 1.3.5 milestone as I think the consensus is for no action.

simonjayhawkins commented 2 years ago

@jbrockmendel if you can repsond to https://github.com/pandas-dev/pandas/issues/43091#issuecomment-961173962 we can probably close this issue. Thanks.

jbrockmendel commented 2 years ago

However, I would also have to check for an inferred type of mixed, for when my column of datetime.date objects contains null dates, right?

I'd go for lib.infer_dtype(col, skipna=True) == "date" instead of checking for "mixed"

jgmarcel commented 2 years ago

However, I would also have to check for an inferred type of mixed, for when my column of datetime.date objects contains null dates, right?

I'd go for lib.infer_dtype(col, skipna=True) == "date" instead of checking for "mixed"

Thank you very much! I was not aware of that function. Much appreciated.