BUG: Fix issue with datetime[ns, tz] input in Block.setitem GH32395 #32479

h-vishal · 2020-03-06T03:08:40Z

closes assignment with datetime64[ns, UTC] raises TypeError #32395
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

doc/source/whatsnew/v1.0.2.rst

pandas/core/internals/blocks.py

pandas/tests/internals/test_internals.py

doc/source/whatsnew/v1.0.2.rst

pandas/core/internals/blocks.py

# Conflicts: # pandas/tests/indexing/test_loc.py

pandas/core/internals/blocks.py

jreback · 2020-03-11T18:29:17Z

#29334 might actually fix this (or if not is a bit cleaner)

TomAugspurger · 2020-03-11T19:30:57Z

#29334 doesn't quite seem to fix this (some of the new tests here fail on that branch).

TomAugspurger · 2020-03-12T13:16:59Z

FYI, I'm currently planning to push this to the next release.

h-vishal · 2020-03-16T15:52:00Z

Any thoughts on - #32479 (comment)?

simonjayhawkins · 2020-03-26T19:18:27Z

#29334 doesn't quite seem to fix this (some of the new tests here fail on that branch).

#29334 has been merged and did not fix.

if this has stalled, is reverting #29139 an option?

@h-vishal can you merge master to resolve conflicts and move whatsnew to 1.1.0

jbrockmendel · 2020-03-26T20:21:27Z

I've been working on Block.setitem, will see if I can find a nice option that doesnt entail reverting #29139.

# Conflicts: # doc/source/whatsnew/v1.0.2.rst # pandas/tests/indexing/test_loc.py

h-vishal · 2020-03-27T18:28:54Z

@simonjayhawkins I have updated this, please look at alternate solutions discussed in #32479 (comment) and #32479 (comment) if one of those is preferred I will push another update.

@jbrockmendel let me know if there is an update on this.

pandas/tests/indexing/test_loc.py

pandas/core/internals/blocks.py

jbrockmendel · 2020-03-28T03:29:48Z

@h-vishal the thing to do here is to add a clause into Block.setitem just after the elif exact_match and is_categorical_dtype(arr_value.dtype) check:

        elif exact_match and is_extension_array_dtype(arr_value.dtype):
            # GH#32395 if we're going to replace the values entirely, just
            #  substitute in the new array
            return self.make_block(arr_value)

Then update the tests to use tm.assert_series_equal and tm.assert_frame_equal as others have suggested.

caveat 1: i havent run the whole test suite on this yet
caveat 2: this works if we are setting obj.loc[:] = other but I havent looked at obj.loc[:halfway] = other[:halfway]

h-vishal · 2020-03-28T15:16:45Z

@jbrockmendel this will create a column with the corresponding datetime64[ns, tz] dtype when a full column assignment occurs so the current tests can be modified to use assert_frame_equals, partial column assignments to datetime64[ns, tz] data would create object dtype columns for which I can add additional tests.

Would it be better to use is_datetime64tz_dtype(arr_value.dtype) instead of is_extension_array_dtype(arr_value.dtype)?

pandas/core/internals/blocks.py

jbrockmendel · 2020-04-04T16:50:34Z

pandas/tests/indexing/test_loc.py

+    s2 = pd.Series(index=s1.index, dtype="object", name="data")
+    expected = pd.Series(
+        [
+            *pd.date_range("2020-01-01", "2020-01-03", 3, tz=timezone.utc),


why the "*"

i guess do

dti = pd.date_range("2020-01-01", "2020-01-03", periods=6, tz=timezone.utc) expected = pd.Series(dti) expected[-3:] = pd.NaT

@jbrockmendel okay, if that is preferred.

@jbrockmendel please note the object dtype for the result of a slice assignement

pandas/tests/computation/test_eval.py

jbrockmendel · 2020-04-04T16:53:27Z

While writing the new tests I came across a bug with a slice assignment of a sparse column

does this PR fix the newly found bug?

h-vishal · 2020-04-04T17:39:02Z

While writing the new tests I came across a bug with a slice assignment of a sparse column

does this PR fix the newly found bug?

@jbrockmendel no, it's unrelated, confirmed in version 0.25 as well, better to create a new issue for it.

tests.computation.test_eval.check_alignment()" This reverts commit 8cfa045

jreback · 2020-04-05T20:05:43Z

doc/source/whatsnew/v1.0.4.rst

@@ -0,0 +1,28 @@
+
+.. _whatsnew_104:


pls remove this and all references to 1.0.4, just put a note in 1.1

jreback · 2020-04-05T20:05:50Z

doc/source/whatsnew/index.rst

@@ -24,6 +24,7 @@ Version 1.0
 .. toctree::
   :maxdepth: 2

+   v1.0.4


revert this

jreback · 2020-04-05T20:05:55Z

doc/source/whatsnew/v1.0.3.rst

@@ -26,4 +26,4 @@ Bug fixes
 Contributors
 ~~~~~~~~~~~~

-.. contributors:: v1.0.2..v1.0.3|HEAD
+.. contributors:: v1.0.2..v1.0.3


revert this

jreback · 2020-04-05T20:07:20Z

pandas/tests/indexing/test_loc.py

+
+
+def test_loc_setitem_df_datetime64tz_full_column_with_index():
+    df = pd.DataFrame(


add a comment with the issue number

jreback · 2020-04-05T20:07:46Z

pandas/tests/indexing/test_loc.py

+    df = pd.DataFrame(
+        pd.date_range("2020-01-01", "2020-01-06", 6, tz=timezone.utc), columns=["data"]
+    )
+    df2 = pd.DataFrame(index=df.index)


use

result =
expected =

as its not really clear what you are comparing here

jreback · 2020-04-05T20:07:56Z

pandas/tests/indexing/test_loc.py

+
+
+def test_loc_setitem_df_datetime64tz_slice():
+    df = pd.DataFrame(


add the issue number as a comment

jreback · 2020-04-05T20:08:21Z

pandas/tests/indexing/test_loc.py

+        columns=["data"],
+        dtype="object",
+    )
+    df2.loc[df.index[:3], "data"] = df["data"][:3]


use
result =
expected =

jreback · 2020-04-05T20:09:26Z

pandas/tests/indexing/test_loc.py

+    tm.assert_frame_equal(df2, expected)
+
+
+def test_loc_setitem_df_datetime64tz_column_without_index():


you don't need to repeat things like this, instead we aleady have generic tests in : pandas/tests/extension//base/setitem.py

@h-vishal this is the key comment. Please do not repeat things like this. We already have fixtures for all of these.

@jreback I have pushed a commit with the requested changes.

it doesn't look like anything has changed. there should be NO tests here, rather in pandas/tests/extension/base/setitem

pandas/core/internals/blocks.py

# Conflicts: # doc/source/whatsnew/v1.1.0.rst

jreback · 2020-04-08T15:27:00Z

pandas/tests/indexing/test_loc.py

@@ -1106,3 +1107,133 @@ def test_loc_with_period_index_indexer():
    tm.assert_frame_equal(df, df.loc[list(idx)])
    tm.assert_frame_equal(df.iloc[0:5], df.loc[idx[0:5]])
    tm.assert_frame_equal(df, df.loc[list(idx)])
+
+
+def test_loc_setitem_df_datetime64tz_full_column_with_index():


all of this need to move to pandas/tests/extension/setitem

BUT you need to use the fixtures, so you are like writing ONE test.

jbrockmendel · 2020-04-17T23:01:28Z

@h-vishal can you rebase and address comments

jreback · 2020-05-25T17:39:50Z

thanks @h-vishal

lumberbot-app · 2020-05-25T17:40:18Z

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

$ git checkout 1.0.x
$ git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

$ git cherry-pick -m1 0f2ca378a2b692edba7baab19ea138b887838cc0

You will likely have some merge/cherry-pick conflict here, fix them and commit:

$ git commit -am 'Backport PR #32479: BUG: Fix issue with datetime[ns, tz] input in Block.setitem GH32395'

Push to a named branch :

git push YOURFORK 1.0.x:auto-backport-of-pr-32479-on-1.0.x

Create a PR against branch 1.0.x, I would have named this PR:

"Backport PR #32479 on branch 1.0.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

…etime[ns, tz] input in Block.setitem)

…tz] input in Block.setitem) (#34369) Co-authored-by: h-vishal <[email protected]> Co-authored-by: jbrockmendel <[email protected]>

BUG: Fix issue with datetime[ns, tz] input in Block.setitem GH32395

198474d

jbrockmendel reviewed Mar 6, 2020

View reviewed changes

doc/source/whatsnew/v1.0.2.rst Outdated Show resolved Hide resolved

jbrockmendel reviewed Mar 6, 2020

View reviewed changes

pandas/core/internals/blocks.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Mar 6, 2020

View reviewed changes

pandas/tests/internals/test_internals.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Mar 6, 2020

View reviewed changes

pandas/tests/internals/test_internals.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Mar 6, 2020

View reviewed changes

pandas/tests/internals/test_internals.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Mar 6, 2020

View reviewed changes

pandas/tests/internals/test_internals.py Outdated Show resolved Hide resolved

h-vishal added 2 commits March 6, 2020 11:37

Added tests for series, moved tests to test_loc.py

6eba1c9

Add dtype to series object in tests to suppress warning

11b4f29

TomAugspurger reviewed Mar 6, 2020

View reviewed changes

doc/source/whatsnew/v1.0.2.rst Outdated Show resolved Hide resolved

pandas/core/internals/blocks.py Outdated Show resolved Hide resolved

h-vishal added 4 commits March 7, 2020 00:13

Fix whatsnew

1b3cba1

Merge remote-tracking branch 'remotes/upstream/master' into issue-32395

d83cff9

Merge remote-tracking branch 'remotes/upstream/master' into issue-32395

4e66228

Merge remote-tracking branch 'remotes/upstream/master' into issue-32395

ac85aa3

# Conflicts: # pandas/tests/indexing/test_loc.py

h-vishal requested review from jbrockmendel, TomAugspurger and WillAyd March 9, 2020 14:08

jbrockmendel reviewed Mar 11, 2020

View reviewed changes

pandas/core/internals/blocks.py Outdated Show resolved Hide resolved

h-vishal added 2 commits March 27, 2020 22:48

Merge remote-tracking branch 'remotes/upstream/master' into issue-32395

2d25c7a

# Conflicts: # doc/source/whatsnew/v1.0.2.rst # pandas/tests/indexing/test_loc.py

Move whatsnew to 1.1.0

3e78985

jreback requested changes Mar 27, 2020

View reviewed changes

pandas/tests/indexing/test_loc.py Outdated Show resolved Hide resolved

pandas/core/internals/blocks.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Apr 4, 2020

View reviewed changes

pandas/core/internals/blocks.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Apr 4, 2020

View reviewed changes

pandas/tests/computation/test_eval.py Outdated Show resolved Hide resolved

h-vishal added 3 commits April 5, 2020 01:28

Revert "TST: Fix precision test in

4f8fccd

tests.computation.test_eval.check_alignment()" This reverts commit 8cfa045

CLN: Test style tests.indexing.test_loc

0f8a913

Merge remote-tracking branch 'remotes/upstream/master' into issue-32395

9d4d1df

jreback requested changes Apr 5, 2020

View reviewed changes

h-vishal added 2 commits April 6, 2020 22:29

Modify tests after review

a01a676

Merge remote-tracking branch 'remotes/upstream/master' into issue-32395

26f8ed3

# Conflicts: # doc/source/whatsnew/v1.1.0.rst

h-vishal requested a review from jreback April 8, 2020 09:35

jreback reviewed Apr 8, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into issue-32395

3caf161

simonjayhawkins added the Still Needs Manual Backport label May 7, 2020

simonjayhawkins added 3 commits May 20, 2020 18:32

Merge remote-tracking branch 'upstream/master' into issue-32395

8d9e6d7

nits

fbc31c8

update tests

c7e7cf5

simonjayhawkins mentioned this pull request May 25, 2020

RLS: 1.0.4 #33300

Closed

jreback approved these changes May 25, 2020

View reviewed changes

jreback added this to the 1.0.4 milestone May 25, 2020

jreback merged commit 0f2ca37 into pandas-dev:master May 25, 2020

simonjayhawkins pushed a commit to simonjayhawkins/pandas that referenced this pull request May 25, 2020

Backport PR pandas-dev#32479 on branch 1.0.x (BUG: Fix issue with dat…

7719aaa

…etime[ns, tz] input in Block.setitem)

simonjayhawkins mentioned this pull request May 25, 2020

Backport PR #32479 on branch 1.0.x (BUG: Fix issue with datetime[ns, tz] input in Block.setitem) #34369

Merged

simonjayhawkins removed the Still Needs Manual Backport label May 25, 2020

simonjayhawkins added a commit that referenced this pull request May 26, 2020

Backport PR #32479 on branch 1.0.x (BUG: Fix issue with datetime[ns, …

d7a3932

…tz] input in Block.setitem) (#34369) Co-authored-by: h-vishal <[email protected]> Co-authored-by: jbrockmendel <[email protected]>

jbrockmendel mentioned this pull request Jan 2, 2021

API: setitem copy/view behavior ndarray vs Categorical vs other EA #38896

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Fix issue with datetime[ns, tz] input in Block.setitem GH32395 #32479

BUG: Fix issue with datetime[ns, tz] input in Block.setitem GH32395 #32479

h-vishal commented Mar 6, 2020

jreback commented Mar 11, 2020

TomAugspurger commented Mar 11, 2020

TomAugspurger commented Mar 12, 2020

h-vishal commented Mar 16, 2020

simonjayhawkins commented Mar 26, 2020

jbrockmendel commented Mar 26, 2020

h-vishal commented Mar 27, 2020

jbrockmendel commented Mar 28, 2020

h-vishal commented Mar 28, 2020

jbrockmendel Apr 4, 2020

jbrockmendel Apr 4, 2020

h-vishal Apr 4, 2020

h-vishal Apr 4, 2020

jbrockmendel commented Apr 4, 2020

h-vishal commented Apr 4, 2020

jreback Apr 5, 2020

jreback Apr 5, 2020

jreback Apr 5, 2020

jreback Apr 5, 2020

jreback Apr 5, 2020

jreback Apr 5, 2020

jreback Apr 5, 2020

jreback Apr 5, 2020

jreback Apr 6, 2020

h-vishal Apr 7, 2020

jreback Apr 8, 2020

jreback Apr 8, 2020

jbrockmendel commented Apr 17, 2020

jreback commented May 25, 2020

lumberbot-app bot commented May 25, 2020



		def test_loc_setitem_df_datetime64tz_full_column_with_index():
		df = pd.DataFrame(



		def test_loc_setitem_df_datetime64tz_slice():
		df = pd.DataFrame(

		tm.assert_frame_equal(df2, expected)


		def test_loc_setitem_df_datetime64tz_column_without_index():

BUG: Fix issue with datetime[ns, tz] input in Block.setitem GH32395 #32479

BUG: Fix issue with datetime[ns, tz] input in Block.setitem GH32395 #32479

Conversation

h-vishal commented Mar 6, 2020

jreback commented Mar 11, 2020

TomAugspurger commented Mar 11, 2020

TomAugspurger commented Mar 12, 2020

h-vishal commented Mar 16, 2020

simonjayhawkins commented Mar 26, 2020

jbrockmendel commented Mar 26, 2020

h-vishal commented Mar 27, 2020

jbrockmendel commented Mar 28, 2020

h-vishal commented Mar 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented Apr 4, 2020

h-vishal commented Apr 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented Apr 17, 2020

jreback commented May 25, 2020

lumberbot-app bot commented May 25, 2020