BUG/TST: PeriodArray.setitem with slice and list-like value #23991

charlesdong1991 · 2018-11-29T10:23:27Z

closes BUG/TST: PeriodArray.__setitem__ with slice and list-like value fails #23978
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

pep8speaks · 2018-11-29T10:23:30Z

Hello @charlesdong1991! Thanks for submitting the PR.

There are no PEP8 issues in the file pandas/tests/arrays/test_period.py !

TomAugspurger

A few things

The tests should be written as base extension tests, see http://pandas-docs.github.io/pandas-docs-travis/extending.html#testing-extension-arrays. These can go in tests/extension/base/setitem.py::BaseSetitemTests.
Extension Arrays define a data fixture. You don't need to define one.
Once you've written the test, pytest pandas/tests/extension/test_period.py::TestSetitem should fail, because of the bug from the original issue.

TomAugspurger · 2018-11-29T14:20:52Z

Great, your test has successfully raised the original bug: https://travis-ci.org/pandas-dev/pandas/jobs/461252693#L2863

So now you'll need to make the changes to PeriodArray.__setitem__ to fix the bug. Make sense?

charlesdong1991 · 2018-11-29T18:05:17Z

thank you very much @TomAugspurger ! this time it should work!

TomAugspurger · 2018-11-29T20:03:48Z

Some linting failures

https://travis-ci.org/pandas-dev/pandas/jobs/461404542#L2851

https://travis-ci.org/pandas-dev/pandas/jobs/461404542#L2868

charlesdong1991 · 2018-11-29T21:57:26Z

HI, @TomAugspurger , ehh, this Flaky error appears again... Is it because my changes didn't pass the hinting test?

codecov · 2018-11-30T12:38:25Z

Codecov Report

Merging #23991 into master will decrease coverage by 49.86%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           master   #23991       +/-   ##
===========================================
- Coverage   92.31%   42.44%   -49.87%     
===========================================
  Files         161      161               
  Lines       51513    51562       +49     
===========================================
- Hits        47554    21888    -25666     
- Misses       3959    29674    +25715

Flag	Coverage Δ
#multiple	`?`
#single	`42.44% <0%> (+0.02%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/arrays/period.py	`37.02% <0%> (-61.42%)`	⬇️
pandas/io/formats/latex.py	`0% <0%> (-100%)`	⬇️
pandas/core/categorical.py	`0% <0%> (-100%)`	⬇️
pandas/io/sas/sas_constants.py	`0% <0%> (-100%)`	⬇️
pandas/tseries/plotting.py	`0% <0%> (-100%)`	⬇️
pandas/tseries/converter.py	`0% <0%> (-100%)`	⬇️
pandas/io/formats/html.py	`0% <0%> (-97.64%)`	⬇️
pandas/core/groupby/categorical.py	`0% <0%> (-95.46%)`	⬇️
pandas/io/sas/sas7bdat.py	`0% <0%> (-91.17%)`	⬇️
pandas/io/sas/sas_xport.py	`0% <0%> (-90.24%)`	⬇️
... and 120 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43b2dab...6c9702e. Read the comment docs.

TomAugspurger · 2018-12-01T16:15:36Z

pandas/tests/extension/test_period.py

@@ -147,7 +148,16 @@ class TestReshaping(BasePeriodTests, base.BaseReshapingTests):


 class TestSetitem(BasePeriodTests, base.BaseSetitemTests):
-    pass
+
+    def test_setitem_slice_mismatch_length_raises(self, data):


Ahh, just noticed. This should be on the base class BaseSetitemTests. Then all the subclasses will be tested, including period.

Same for the other test.

ahh, i see, yeah, makes sense!! thx!

TomAugspurger · 2018-12-01T22:54:04Z

pandas/tests/extension/test_period.py

@@ -8,6 +8,7 @@
 import pandas as pd
 from pandas.core.arrays import PeriodArray
 from pandas.tests.extension import base
+import pandas.util.testing as tm


Probably delete this

TomAugspurger · 2018-12-01T22:54:12Z

pandas/tests/extension/test_period.py

@@ -147,6 +148,7 @@ class TestReshaping(BasePeriodTests, base.BaseReshapingTests):


 class TestSetitem(BasePeriodTests, base.BaseSetitemTests):
+


jreback

needs a whatsnew entry

jreback · 2018-12-02T23:19:10Z

thanks @charlesdong1991

…as-dev#23991)

pytest for slice type in setitem

4385128

TomAugspurger reviewed Nov 29, 2018

View reviewed changes

TomAugspurger added Testing pandas testing functions or related to the test suite Period Period data type labels Nov 29, 2018

charlesdong1991 added 2 commits November 29, 2018 13:26

remove wrong pytests

aacb769

new pytest in TestSetitem

e19c381

fix pytest error

b12f0bc

charlesdong1991 changed the title ~~pytest for slice type in setitem~~ BUG/TST: PeriodArray.__setitem__ with slice and list-like value Nov 29, 2018

charlesdong1991 added 2 commits November 29, 2018 21:12

remove a blank line to see if error disappear

6ca1f1a

resort imports

f3db6c1

charlesdong1991 added 5 commits November 30, 2018 09:20

fix linting failure

fe74ea1

fix linting error

0e19fdb

resort imports

3e76a61

resort imports using isort

ec8f68e

remove redundant blank line

217146f

TomAugspurger added this to the 0.24.0 milestone Dec 1, 2018

TomAugspurger reviewed Dec 1, 2018

View reviewed changes

move to basesetitem

74ea7b8

TomAugspurger reviewed Dec 1, 2018

View reviewed changes

remove unused package from import

39a4807

jreback requested changes Dec 2, 2018

View reviewed changes

add whatsnew description

6c9702e

jreback approved these changes Dec 2, 2018

View reviewed changes

jreback merged commit d967aef into pandas-dev:master Dec 2, 2018

jreback mentioned this pull request Dec 2, 2018

TST: run setitem tests for IntegerArray #24054

Merged

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

BUG/TST: PeriodArray.__setitem__ with slice and list-like value (pand…

a1873dc

…as-dev#23991)

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

BUG/TST: PeriodArray.__setitem__ with slice and list-like value (pand…

e5d9ecd

…as-dev#23991)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG/TST: PeriodArray.setitem with slice and list-like value #23991

BUG/TST: PeriodArray.setitem with slice and list-like value #23991

charlesdong1991 commented Nov 29, 2018 •

edited

Loading

pep8speaks commented Nov 29, 2018

TomAugspurger left a comment

TomAugspurger commented Nov 29, 2018

charlesdong1991 commented Nov 29, 2018

TomAugspurger commented Nov 29, 2018

charlesdong1991 commented Nov 29, 2018

codecov bot commented Nov 30, 2018 •

edited

Loading

TomAugspurger Dec 1, 2018

TomAugspurger Dec 1, 2018

charlesdong1991 Dec 1, 2018

TomAugspurger Dec 1, 2018

TomAugspurger Dec 1, 2018

jreback left a comment

jreback commented Dec 2, 2018

		@@ -147,6 +148,7 @@ class TestReshaping(BasePeriodTests, base.BaseReshapingTests):


		class TestSetitem(BasePeriodTests, base.BaseSetitemTests):

BUG/TST: PeriodArray.__setitem__ with slice and list-like value #23991

BUG/TST: PeriodArray.__setitem__ with slice and list-like value #23991

Conversation

charlesdong1991 commented Nov 29, 2018 • edited Loading

pep8speaks commented Nov 29, 2018

TomAugspurger left a comment

Choose a reason for hiding this comment

TomAugspurger commented Nov 29, 2018

charlesdong1991 commented Nov 29, 2018

TomAugspurger commented Nov 29, 2018

charlesdong1991 commented Nov 29, 2018

codecov bot commented Nov 30, 2018 • edited Loading

Codecov Report

TomAugspurger Dec 1, 2018

Choose a reason for hiding this comment

TomAugspurger Dec 1, 2018

Choose a reason for hiding this comment

charlesdong1991 Dec 1, 2018

Choose a reason for hiding this comment

TomAugspurger Dec 1, 2018

Choose a reason for hiding this comment

TomAugspurger Dec 1, 2018

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

jreback commented Dec 2, 2018

BUG/TST: PeriodArray.setitem with slice and list-like value #23991

BUG/TST: PeriodArray.setitem with slice and list-like value #23991

charlesdong1991 commented Nov 29, 2018 •

edited

Loading

codecov bot commented Nov 30, 2018 •

edited

Loading