Add base test for extensionarray setitem #23300 #23304

charlesdong1991 · 2018-10-23T19:17:42Z

closes Add base test for ExtensionArray.__setitem__ #23300
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

pep8speaks · 2018-10-23T19:17:44Z

Hello @charlesdong1991! Thanks for submitting the PR.

There are no PEP8 issues in the file pandas/conftest.py !
There are no PEP8 issues in the file pandas/tests/extension/base/setitem.py !

TomAugspurger · 2018-10-23T19:22:42Z

pandas/tests/extension/base/setitem.py

-    def test_setitem_scalar(self, data, setter):
-        arr = pd.Series(data)
-        setter = getattr(arr, setter)
+    def test_setitem_scalar(self, data, box, setter):


I think this will not work, since loc and iloc are only available on Series.

changed.
thanks for the quick feedback!

pandas/tests/extension/base/setitem.py

TomAugspurger · 2018-10-23T20:55:40Z

Looks like the fixture name box is already taken

    raise ValueError("duplicate %r" % (arg,))
E   ValueError: duplicate 'box'
--------------------- generated xml file: /tmp/single.xml ----------------------
!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!
============ 7 skipped, 35608 deselected, 1 error in 35.79 seconds =============

You could rename it to box_in_series perhaps.

FYI, you may want to run the tests locally for a faster feedback loop. pytest pandas/tests/extension or even just pytest pandas/tests/extension/decimal.

jreback · 2018-10-24T12:55:26Z

pandas/conftest.py

@@ -501,6 +501,12 @@ def mock():
        return pytest.importorskip("mock")


+@pytest.fixture(params=[True, False])
+def box_in_series(request):


put this in the extension conftest

done, thanks!

pandas/tests/extension/base/setitem.py

jreback · 2018-10-26T00:36:03Z

looks good.

@TomAugspurger IIRC on Sparse you were showing a warning on setitem (for perf)? is this tested ?

codecov · 2018-10-26T01:00:49Z

Codecov Report

❗ No coverage uploaded for pull request base (master@4a11eac). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #23304   +/-   ##
=========================================
  Coverage          ?   92.23%           
=========================================
  Files             ?      169           
  Lines             ?    50925           
  Branches          ?        0           
=========================================
  Hits              ?    46969           
  Misses            ?     3956           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.65% <ø> (?)`
#single	`42.28% <ø> (?)`

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 4a11eac...009440d. Read the comment docs.

TomAugspurger · 2018-10-26T01:38:08Z

SparseArray doesn't implement __setitem__ (old one didn't either, though there's an open PR to implement it).

TomAugspurger · 2018-10-26T01:38:24Z

Thanks @charlesdong1991!

jorisvandenbossche · 2018-10-26T09:18:19Z

pandas/tests/extension/base/setitem.py

@@ -32,22 +34,25 @@ def test_setitem_sequence_mismatched_length_raises(self, data, as_array):
        xpr = 'cannot set using a {} indexer with a different length'
        with tm.assert_raises_regex(ValueError, xpr.format('list-like')):
            ser[[0, 1]] = value
+            assert ser._values[[0, 1]] == value


@TomAugspurger how will this assert be reached if the previous line raises an error?

Ah, sorry I missed that.

charlesdong1991 · 2018-10-26T12:11:23Z

ah, sorry, i see, thanks for pointing it out... can PR be reopened? Tom Augspurger <notifications@github.com> 于2018年10月26日周五下午1:11写道：

…

***@***.**** commented on this pull request. ------------------------------ In pandas/tests/extension/base/setitem.py <#23304 (comment)>: > @@ -32,22 +34,25 @@ def test_setitem_sequence_mismatched_length_raises(self, data, as_array): xpr = 'cannot set using a {} indexer with a different length' with tm.assert_raises_regex(ValueError, xpr.format('list-like')): ser[[0, 1]] = value + assert ser._values[[0, 1]] == value Ah, sorry I missed that. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#23304 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AI1yOGQ5uaG34l6w7Z-FoUOuyL5dF4yHks5uou3ggaJpZM4X2dmH> .

TomAugspurger · 2018-10-26T12:12:49Z

No worries. I made a new PR to fix it.

…

On Fri, Oct 26, 2018 at 7:11 AM Kaiqi Dong ***@***.***> wrote: ah, sorry, i see, thanks for pointing it out... can PR be reopened? Tom Augspurger ***@***.***> 于2018年10月26日周五下午1:11写道： > ***@***.**** commented on this pull request. > ------------------------------ > > In pandas/tests/extension/base/setitem.py > <#23304 (comment)>: > > > @@ -32,22 +34,25 @@ def test_setitem_sequence_mismatched_length_raises(self, data, as_array): > xpr = 'cannot set using a {} indexer with a different length' > with tm.assert_raises_regex(ValueError, xpr.format('list-like')): > ser[[0, 1]] = value > + assert ser._values[[0, 1]] == value > > Ah, sorry I missed that. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#23304 (comment)>, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AI1yOGQ5uaG34l6w7Z-FoUOuyL5dF4yHks5uou3ggaJpZM4X2dmH > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#23304 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIuddzCfKW5Ial4NgWdsKkwH38W2Vks5uovvxgaJpZM4X2dmH> .

charlesdong1991 · 2018-10-26T12:20:36Z

thanks! and appreciate a lot for your patience, guidance and kindness. it's my first time to try to contribute to such big open source project, and still feel excited and looking forward to doing more... wish i could make less trouble for all of you in next PR... ^^ Tom Augspurger <notifications@github.com> 于2018年10月26日周五下午2:13写道：

…

No worries. I made a new PR to fix it. On Fri, Oct 26, 2018 at 7:11 AM Kaiqi Dong ***@***.***> wrote: > ah, sorry, i see, thanks for pointing it out... can PR be reopened? > > Tom Augspurger ***@***.***> 于2018年10月26日周五下午1:11写道： > > > ***@***.**** commented on this pull request. > > ------------------------------ > > > > In pandas/tests/extension/base/setitem.py > > <#23304 (comment) >: > > > > > @@ -32,22 +34,25 @@ def > test_setitem_sequence_mismatched_length_raises(self, data, as_array): > > xpr = 'cannot set using a {} indexer with a different length' > > with tm.assert_raises_regex(ValueError, xpr.format('list-like')): > > ser[[0, 1]] = value > > + assert ser._values[[0, 1]] == value > > > > Ah, sorry I missed that. > > > > — > > You are receiving this because you were mentioned. > > Reply to this email directly, view it on GitHub > > <#23304 (comment) >, > > or mute the thread > > < > https://github.com/notifications/unsubscribe-auth/AI1yOGQ5uaG34l6w7Z-FoUOuyL5dF4yHks5uou3ggaJpZM4X2dmH > > > > . > > > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#23304 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/ABQHIuddzCfKW5Ial4NgWdsKkwH38W2Vks5uovvxgaJpZM4X2dmH > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#23304 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AI1yOIQjwLFzgvsCtCJPlXGfAaEHXTspks5uovxYgaJpZM4X2dmH> .

…_pr2 * repo_org/master: DOC: Add docstring validations for "See Also" section (pandas-dev#23143) TST: Fix test assertion (pandas-dev#23357) BUG: Handle Period in combine (pandas-dev#23350) REF: SparseArray imports (pandas-dev#23329) CI: Migrate some CircleCI jobs to Azure (pandas-dev#22992) DOC: update the is_month_start/is_month_end docstring (pandas-dev#23051) Partialy fix issue pandas-dev#23334 - isort pandas/core/groupby directory (pandas-dev#23341) TST: Add base test for extensionarray setitem pandas-dev#23300 (pandas-dev#23304) API: Add sparse Acessor (pandas-dev#23183) PERF: speed up CategoricalIndex.get_loc (pandas-dev#23235)

…ndas * repo_org/master: (23 commits) DOC: Add docstring validations for "See Also" section (pandas-dev#23143) TST: Fix test assertion (pandas-dev#23357) BUG: Handle Period in combine (pandas-dev#23350) REF: SparseArray imports (pandas-dev#23329) CI: Migrate some CircleCI jobs to Azure (pandas-dev#22992) DOC: update the is_month_start/is_month_end docstring (pandas-dev#23051) Partialy fix issue pandas-dev#23334 - isort pandas/core/groupby directory (pandas-dev#23341) TST: Add base test for extensionarray setitem pandas-dev#23300 (pandas-dev#23304) API: Add sparse Acessor (pandas-dev#23183) PERF: speed up CategoricalIndex.get_loc (pandas-dev#23235) fix and test incorrect case in delta_to_nanoseconds (pandas-dev#23302) BUG: Handle Datetimelike data in DataFrame.combine (pandas-dev#23317) TST: re-enable gbq tests (pandas-dev#23303) Switched references of App veyor to azure pipelines in the contributing CI section (pandas-dev#23311) isort imports-io (pandas-dev#23332) DOC: Added a Multi Index example for the Series.sum method (pandas-dev#23279) REF: Make PeriodArray an ExtensionArray (pandas-dev#22862) DOC: Added Examples for Series max (pandas-dev#23298) API/ENH: tz_localize handling of nonexistent times: rename keyword + add shift option (pandas-dev#22644) BUG: Let MultiIndex.set_levels accept any iterable (pandas-dev#23273) (pandas-dev#23291) ...

…xamples * repo_org/master: (83 commits) DOC: Add docstring validations for "See Also" section (pandas-dev#23143) TST: Fix test assertion (pandas-dev#23357) BUG: Handle Period in combine (pandas-dev#23350) REF: SparseArray imports (pandas-dev#23329) CI: Migrate some CircleCI jobs to Azure (pandas-dev#22992) DOC: update the is_month_start/is_month_end docstring (pandas-dev#23051) Partialy fix issue pandas-dev#23334 - isort pandas/core/groupby directory (pandas-dev#23341) TST: Add base test for extensionarray setitem pandas-dev#23300 (pandas-dev#23304) API: Add sparse Acessor (pandas-dev#23183) PERF: speed up CategoricalIndex.get_loc (pandas-dev#23235) fix and test incorrect case in delta_to_nanoseconds (pandas-dev#23302) BUG: Handle Datetimelike data in DataFrame.combine (pandas-dev#23317) TST: re-enable gbq tests (pandas-dev#23303) Switched references of App veyor to azure pipelines in the contributing CI section (pandas-dev#23311) isort imports-io (pandas-dev#23332) DOC: Added a Multi Index example for the Series.sum method (pandas-dev#23279) REF: Make PeriodArray an ExtensionArray (pandas-dev#22862) DOC: Added Examples for Series max (pandas-dev#23298) API/ENH: tz_localize handling of nonexistent times: rename keyword + add shift option (pandas-dev#22644) BUG: Let MultiIndex.set_levels accept any iterable (pandas-dev#23273) (pandas-dev#23291) ...

…s-dev#23304)

charlesdong1991 added 2 commits October 23, 2018 21:06

add params pytest fixture

d9a150c

add box to be able to directly test setitem

ddce1fe

TomAugspurger reviewed Oct 23, 2018

View reviewed changes

pandas/tests/extension/base/setitem.py Show resolved Hide resolved

TomAugspurger added ExtensionArray Extending pandas with custom dtypes or arrays. Testing pandas testing functions or related to the test suite labels Oct 23, 2018

TomAugspurger added this to the 0.24.0 milestone Oct 23, 2018

charlesdong1991 added 2 commits October 23, 2018 21:25

remove box in test_setitem_scalar function

84800f8

add two asserts

9350641

charlesdong1991 added 8 commits October 24, 2018 11:53

rename fixture

f12ffd2

rename box with box_in_series

d50715b

add params pytest fixture

8a9f494

add box to be able to directly test setitem

c2e5895

remove box in test_setitem_scalar function

9bd1919

add two asserts

f4854d8

rename fixture

e544d5e

rename box with box_in_series

d223290

TomAugspurger mentioned this pull request Oct 24, 2018

Add base test for ExtensionArray.__setitem__ #23300

Closed

add ._values for ser

d92988c

charlesdong1991 force-pushed the add_base_test_for_extensionarray_setitem branch from d50715b to d92988c Compare October 24, 2018 12:34

charlesdong1991 added 4 commits October 24, 2018 14:42

remove conflicts

c1f6ebb

debug series type error

fca3b54

debug assert error

10cca66

debug assert error

39df34e

jreback requested changes Oct 24, 2018

View reviewed changes

move fixture from conftest to extension/conftest

9726198

TomAugspurger reviewed Oct 24, 2018

View reviewed changes

pandas/tests/extension/base/setitem.py Show resolved Hide resolved

pandas/tests/extension/base/setitem.py Show resolved Hide resolved

modify identation and add two asserts

462fd3e

charlesdong1991 added 2 commits October 25, 2018 16:52

add two asserts

0619752

fix empty list error

009440d

jreback approved these changes Oct 26, 2018

View reviewed changes

TomAugspurger merged commit 6703ace into pandas-dev:master Oct 26, 2018

jorisvandenbossche reviewed Oct 26, 2018

View reviewed changes

TomAugspurger mentioned this pull request Oct 26, 2018

TST: Fix test assertion #23357

Merged

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018

TST: Add base test for extensionarray setitem pandas-dev#23300 (panda…

3fac4d2

…s-dev#23304)

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

TST: Add base test for extensionarray setitem pandas-dev#23300 (panda…

7b6b247

…s-dev#23304)

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

TST: Add base test for extensionarray setitem pandas-dev#23300 (panda…

4e12652

…s-dev#23304)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add base test for extensionarray setitem #23300 #23304

Add base test for extensionarray setitem #23300 #23304

charlesdong1991 commented Oct 23, 2018 •

edited

Loading

pep8speaks commented Oct 23, 2018

TomAugspurger Oct 23, 2018

charlesdong1991 Oct 23, 2018

TomAugspurger commented Oct 23, 2018

jreback Oct 24, 2018

charlesdong1991 Oct 24, 2018

jreback commented Oct 26, 2018 •

edited

Loading

codecov bot commented Oct 26, 2018 •

edited

Loading

TomAugspurger commented Oct 26, 2018

TomAugspurger commented Oct 26, 2018

jorisvandenbossche Oct 26, 2018

TomAugspurger Oct 26, 2018

charlesdong1991 commented Oct 26, 2018 via email

TomAugspurger commented Oct 26, 2018 via email

charlesdong1991 commented Oct 26, 2018 via email

Add base test for extensionarray setitem #23300 #23304

Add base test for extensionarray setitem #23300 #23304

Conversation

charlesdong1991 commented Oct 23, 2018 • edited Loading

pep8speaks commented Oct 23, 2018

TomAugspurger Oct 23, 2018

Choose a reason for hiding this comment

charlesdong1991 Oct 23, 2018

Choose a reason for hiding this comment

TomAugspurger commented Oct 23, 2018

jreback Oct 24, 2018

Choose a reason for hiding this comment

charlesdong1991 Oct 24, 2018

Choose a reason for hiding this comment

jreback commented Oct 26, 2018 • edited Loading

codecov bot commented Oct 26, 2018 • edited Loading

Codecov Report

TomAugspurger commented Oct 26, 2018

TomAugspurger commented Oct 26, 2018

jorisvandenbossche Oct 26, 2018

Choose a reason for hiding this comment

TomAugspurger Oct 26, 2018

Choose a reason for hiding this comment

charlesdong1991 commented Oct 26, 2018 via email

TomAugspurger commented Oct 26, 2018 via email

charlesdong1991 commented Oct 26, 2018 via email

charlesdong1991 commented Oct 23, 2018 •

edited

Loading

jreback commented Oct 26, 2018 •

edited

Loading

codecov bot commented Oct 26, 2018 •

edited

Loading