ibis icon indicating copy to clipboard operation
ibis copied to clipboard

[10.0] fix: return `pd.Timestamp` or `pd.Series[datetime64]` for `date.to_pandas()`

Open mfatihaktas opened this issue 1 year ago • 5 comments

Description of changes

Aims to close https://github.com/ibis-project/ibis/issues/8019.

The example given by @NickCrews in the Issue description runs as follows with the changes here:

>>> d = ibis.memtable({"date": ["2024-01-01", "2024-01-02"]}).cast({"date": "date"}).date
>>> s = d.to_pandas()
>>> print(type(s[0]))
<class 'pandas._libs.tslibs.timestamps.Timestamp'>
>>> s
0   2024-01-01
1   2024-01-02
Name: date, dtype: datetime64[s]

To the reviewer: @NickCrews added the following suggestion in the issue:

It seems like you really thought about the semantics of this already per some comments in that issue and linked PR, but curious if there would be a problem with these semantics: DateScalar.to_pandas() -> pd.Timestamp DateColumn.to_pandas() -> pd.Series[datetime64] DateScalar.execute() -> pd.Timestamp (because by definition this uses pandas) DateColumn.execute() -> pd.Series[datetime64] (same) repr(Date) -> "YYYY-MM-DD" (even if under the hood this uses .to_pandas() and we have pd.Timestamps in intermediate steps)

Should we add new tests to verify these suggestions?

mfatihaktas avatar Mar 26 '24 21:03 mfatihaktas

@mfatihaktas What's the status of this one? would you mind fixing the conflicts and triggering CI to see if things are green and request a review.

ncclementi avatar Apr 30 '24 20:04 ncclementi

@mfatihaktas What's the status of this one? would you mind fixing the conflicts and triggering CI to see if things are green and request a review.

Thanks for checking on it. Regarding the status, I marked this PR as ready-for-review some time ago. I am busy with another project this week, but will try to resolve the conflicts before EOD on Friday.

mfatihaktas avatar May 02 '24 13:05 mfatihaktas

I think this PR is good to go, but it's slated for 10.0, so please do not merge.

cpcloud avatar Jul 01 '24 12:07 cpcloud

I made the PR title non-compliant with conventional commits, so we can avoid accidentally clicking the big green button.

gforsyth avatar Jul 01 '24 13:07 gforsyth

ACTION NEEDED

Ibis follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message.

Please update your PR title and description to match the specification.

github-actions[bot] avatar Jul 01 '24 13:07 github-actions[bot]