REGR: allow reindexing datetimelike with upcast / raise deprecation warning #44839

jorisvandenbossche · 2021-12-10T10:38:28Z

Fixes inconsistency/regression in the deprecation of date coercion (DEPR: casting date to dt64 in maybe_promote #39767) -> now this is also done for single-axis reindexing (xref PERF: avoid going through .values for reindexing both index/columns for ArrayManager #44798 (comment))
This also fixes the regression reported in BUG: Reindex null timedelta series with pd.Timedelta fails #42921 it is not fully clear if we really want to "fix" this regression)

…arning

simonjayhawkins · 2021-12-10T12:01:28Z

just to be clear...

s.reindex(
    pd.date_range("2020-01-01", "2020-01-04", freq="1d"), fill_value=pd.Timedelta(0)
)

on 1.2.5

2020-01-01               None
2020-01-02               None
2020-01-03               None
2020-01-04    0 days 00:00:00
Freq: D, dtype: object

and with this PR

2020-01-01                NaT
2020-01-02                NaT
2020-01-03                NaT
2020-01-04    0 days 00:00:00
Freq: D, dtype: object

so this is not restoring 1.2.5 behaviour exactly.

However, the dtype is still object, whereas in the issue OP the expected behavior is for timedelta64[ns] (which is the result when dtype is explictly specfied, second part of #42921 (comment))

so maybe should not yet close #42921 with this PR?

jreback · 2021-12-10T23:56:37Z

moving this to 1.4. we don't need to backport this

…depr

jorisvandenbossche · 2021-12-11T10:29:15Z

I moved this back to 1.3.5 milestone for now (let's discuss first before moving).

It does fix a regression from 1.2->1.3. Code that worked on 1.2, is raising an error on 1.3.

@simonjayhawkins you're right that None vs NaT is not exactly the same (I didn't notice that yet, can look into why that is the case), but it's also a much smaller difference compared to raising an error in 1.3

However, the dtype is still object, whereas in the issue OP the expected behavior is for timedelta64[ns] (which is the result when dtype is explictly specfied, second part of #42921 (comment))

As you explained in that issue, it is expected that this doesn't give timedelta64[ns], since the example starts with a datetime64 dtype.

simonjayhawkins · 2021-12-11T10:50:24Z

As you explained in that issue, it is expected that this doesn't give timedelta64[ns], since the example starts with a datetime64 dtype.

That's fine. we probably need to repeat/confirm this in the issue (that the expected in the OP is incorrect) so that it can be closed off.

jreback · 2021-12-11T13:07:21Z

@jorisvandenbossche it may be a regression but this is delaying the long delayed release

jreback · 2021-12-11T13:07:35Z

it's simply too late

jreback · 2021-12-11T14:23:25Z

@jorisvandenbossche pls don't move this back
it is too late for this patch which hasn't been looked at and will not before release

jbrockmendel · 2021-12-13T16:01:26Z

will cases that go through ExtensionBlock.take_nd be affected/corrected?

jreback · 2021-12-14T01:44:04Z

doc/source/whatsnew/v1.3.5.rst

@@ -21,6 +21,7 @@ Fixed regressions
 - Fixed performance regression in :func:`read_csv` (:issue:`44106`)
 - Fixed regression in :meth:`Series.duplicated` and :meth:`Series.drop_duplicates` when Series has :class:`Categorical` dtype with boolean categories (:issue:`44351`)
 - Fixed regression in :meth:`.GroupBy.sum` with ``timedelta64[ns]`` dtype containing ``NaT`` failing to treat that value as NA (:issue:`42659`)
+- Fixed regression in :meth:`~Series.reindex` raising an error when using an incompatible fill value with a datetime-like dtype (or not raising a deprecation warning for using a ``datetime.date`` as fill value) (:issue:`42921`)


move to 1.4

jreback · 2021-12-14T01:44:32Z

pandas/tests/frame/methods/test_reindex.py

@@ -113,6 +116,11 @@ def test_reindex_date_fill_value(self):
        )
        tm.assert_frame_equal(res, expected)

+        # only reindexing rows
+        with tm.assert_produces_warning(FutureWarning):


why is this a warn?

xref #44798 (comment).

@jorisvandenbossche maybe open dedicated issue for discussion/visibility.

I think this PR can serve that purpose?

could use a match=... to clarify

jorisvandenbossche · 2021-12-14T16:24:55Z

will cases that go through ExtensionBlock.take_nd be affected/corrected?

No, because they are already strict in preserving the dtype anyway

jbrockmendel · 2021-12-14T23:00:09Z

will cases that go through ExtensionBlock.take_nd be affected/corrected?

No, because they are already strict in preserving the dtype anyway

It isn't clear to me that this strictness is correct. i.e. in test_reindex_fill_value_datetimelike_upcast what happens if you add a couple of EA dtypes to the 'dtype' param?

jorisvandenbossche · 2021-12-15T16:17:57Z

It isn't clear to me that this strictness is correct.

It has been like that for a long time (for ever?), and changing (or discussing) that is out of scope for this PR I think.

jbrockmendel · 2021-12-15T22:58:47Z

that is out of scope for this PR I think.

Sure.

Looks like ArrayManager tests are failing.

jbrockmendel · 2021-12-23T23:45:05Z

pandas/core/array_algos/take.py

@@ -94,6 +94,12 @@ def take_nd(
    """
    if fill_value is lib.no_default:
        fill_value = na_value_for_dtype(arr.dtype, compat=False)
+    elif isinstance(arr.dtype, np.dtype) and arr.dtype.kind in "mM":


does take_1d need to be changed to match?

jbrockmendel · 2021-12-27T00:15:35Z

@jorisvandenbossche needs rebase

jreback · 2021-12-31T22:07:39Z

@phofl @jbrockmendel if one of you could rebase this

…depr # Conflicts: # doc/source/whatsnew/v1.4.0.rst

phofl · 2021-12-31T22:44:37Z

rebased

jreback · 2022-01-01T00:01:59Z

@jbrockmendel ok here?

jbrockmendel · 2022-01-01T00:07:48Z

Nothing that should block this. I'd like to get a response here https://github.com/pandas-dev/pandas/pull/44839/files#r774813668 eventually

phofl · 2022-01-01T22:58:29Z

The array_manager runs through take_1d which does not promote to object in case of incompatible fill values. Not familiar enough with the logic to decide on a fix here

jbrockmendel · 2022-01-04T00:38:39Z

The array_manager runs through take_1d which does not promote to object in case of incompatible fill values. Not familiar enough with the logic to decide on a fix here

Yah, trying to make AM behavior consistent is a fool's errand.

jreback · 2022-01-05T23:31:22Z

@jbrockmendel or @phofl can you rebase this

jreback · 2022-01-08T15:23:11Z

@jbrockmendel or @phofl if you can rebase this

jbrockmendel · 2022-01-08T16:36:28Z

merged master

jreback · 2022-01-16T16:59:19Z

hmm a code check / pre-commit issue if you can fixup @jbrockmendel

…h upcast / raise deprecation warning

jreback · 2022-01-16T23:39:07Z

@meeseeksdev backport 1.4.x

lumberbot-app · 2022-01-16T23:39:12Z

Something went wrong ... Please have a look at my logs.

… raise deprecation warning (#45406) Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>

jorisvandenbossche added 3 commits December 10, 2021 10:55

REGR: allow reindexing datetimelike with upcast / raise deprecation w…

c8276c7

…arning

add test for pandas-devGH-42921

f35a3ef

add whatsnew

92c1fe4

jorisvandenbossche added this to the 1.3.5 milestone Dec 10, 2021

jorisvandenbossche requested review from jbrockmendel and simonjayhawkins December 10, 2021 10:38

jreback modified the milestones: 1.3.5, 1.4 Dec 10, 2021

jorisvandenbossche modified the milestones: 1.4, 1.3.5 Dec 11, 2021

jorisvandenbossche added 2 commits December 11, 2021 11:21

maybe_promote is only for np.dtype

2fd957c

Merge remote-tracking branch 'upstream/master' into reindex-datetime-…

d90cf97

…depr

jreback modified the milestones: 1.3.5, 1.4 Dec 11, 2021

jorisvandenbossche modified the milestones: 1.4, 1.3.5 Dec 11, 2021

skip array manager

c4b9ba7

jreback modified the milestones: 1.3.5, 1.4 Dec 11, 2021

jreback requested changes Dec 14, 2021

View reviewed changes

jreback added the Deprecate Functionality to remove in pandas label Dec 14, 2021

jbrockmendel reviewed Dec 23, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/master' into reindex-datetime-…

d100489

…depr # Conflicts: # doc/source/whatsnew/v1.4.0.rst

Try fix error

872ca45

Adjust test

1139b5f

Merge branch 'master' into reindex-datetime-depr

11d8177

jreback approved these changes Jan 4, 2022

View reviewed changes

Merge branch 'master' into reindex-datetime-depr

fda6d51

jorisvandenbossche added 2 commits January 16, 2022 18:22

Merge remote-tracking branch 'upstream/main' into reindex-datetime-depr

914b2af

fix linting

8eb4d59

jreback added the Indexing Related to indexing on series/frames, not to indexes themselves label Jan 16, 2022

jreback merged commit adec4fe into pandas-dev:main Jan 16, 2022

meeseeksmachine mentioned this pull request Jan 16, 2022

Backport PR #44839 on branch 1.4.x (REGR: allow reindexing datetimelike with upcast / raise deprecation warning) #45406

Merged

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Jan 16, 2022

Backport PR pandas-dev#44839: REGR: allow reindexing datetimelike wit…

8a3360c

…h upcast / raise deprecation warning

simonjayhawkins pushed a commit that referenced this pull request Jan 17, 2022

Backport PR #44839: REGR: allow reindexing datetimelike with upcast /…

4354b66

… raise deprecation warning (#45406) Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

REGR: allow reindexing datetimelike with upcast / raise deprecation warning #44839

REGR: allow reindexing datetimelike with upcast / raise deprecation warning #44839

jorisvandenbossche commented Dec 10, 2021

simonjayhawkins commented Dec 10, 2021

jreback commented Dec 10, 2021

jorisvandenbossche commented Dec 11, 2021

simonjayhawkins commented Dec 11, 2021

jreback commented Dec 11, 2021

jreback commented Dec 11, 2021

jreback commented Dec 11, 2021

jbrockmendel commented Dec 13, 2021

jreback Dec 14, 2021

jreback Dec 14, 2021

simonjayhawkins Dec 14, 2021

jorisvandenbossche Dec 14, 2021

jbrockmendel Jan 1, 2022

jorisvandenbossche commented Dec 14, 2021

jbrockmendel commented Dec 14, 2021

jorisvandenbossche commented Dec 15, 2021

jbrockmendel commented Dec 15, 2021

jbrockmendel Dec 23, 2021

jbrockmendel commented Dec 27, 2021

jreback commented Dec 31, 2021

phofl commented Dec 31, 2021

jreback commented Jan 1, 2022

jbrockmendel commented Jan 1, 2022

phofl commented Jan 1, 2022

jbrockmendel commented Jan 4, 2022

jreback commented Jan 5, 2022

jreback commented Jan 8, 2022

jbrockmendel commented Jan 8, 2022

jreback commented Jan 16, 2022

jreback commented Jan 16, 2022

lumberbot-app bot commented Jan 16, 2022

REGR: allow reindexing datetimelike with upcast / raise deprecation warning #44839

REGR: allow reindexing datetimelike with upcast / raise deprecation warning #44839

Conversation

jorisvandenbossche commented Dec 10, 2021

simonjayhawkins commented Dec 10, 2021

jreback commented Dec 10, 2021

jorisvandenbossche commented Dec 11, 2021

simonjayhawkins commented Dec 11, 2021

jreback commented Dec 11, 2021

jreback commented Dec 11, 2021

jreback commented Dec 11, 2021

jbrockmendel commented Dec 13, 2021

jreback Dec 14, 2021

Choose a reason for hiding this comment

jreback Dec 14, 2021

Choose a reason for hiding this comment

simonjayhawkins Dec 14, 2021

Choose a reason for hiding this comment

jorisvandenbossche Dec 14, 2021

Choose a reason for hiding this comment

jbrockmendel Jan 1, 2022

Choose a reason for hiding this comment

jorisvandenbossche commented Dec 14, 2021

jbrockmendel commented Dec 14, 2021

jorisvandenbossche commented Dec 15, 2021

jbrockmendel commented Dec 15, 2021

jbrockmendel Dec 23, 2021

Choose a reason for hiding this comment

jbrockmendel commented Dec 27, 2021

jreback commented Dec 31, 2021

phofl commented Dec 31, 2021

jreback commented Jan 1, 2022

jbrockmendel commented Jan 1, 2022

phofl commented Jan 1, 2022

jbrockmendel commented Jan 4, 2022

jreback commented Jan 5, 2022

jreback commented Jan 8, 2022

jbrockmendel commented Jan 8, 2022

jreback commented Jan 16, 2022

jreback commented Jan 16, 2022

lumberbot-app bot commented Jan 16, 2022