REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py #24053

simonjayhawkins · 2018-12-02T20:26:13Z

xref #24040

for this pass:

the Test classes have been removed.
catch_warnings used for the .ix deprecation replaced with @pytest.mark.filterwarnings("ignore:\\n.ix:DeprecationWarning") at test level.

cc @jreback @gfyoung

pep8speaks · 2018-12-02T20:26:15Z

Hello @simonjayhawkins! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/tests/indexing/multiindex/test_getitem.py !

Comment last updated on December 10, 2018 at 16:06 Hours UTC

gfyoung · 2018-12-02T20:41:31Z

pandas/tests/indexing/multiindex/test_getitem.py

+    result = s.xs('one', level='L2')
+    expected = Series([1, 3], index=['a', 'b'])
+    expected.index.set_names(['L1'], inplace=True)
+    tm.assert_series_equal(result, expected)


Can we break this massive test up into smaller ones?

jreback · 2018-12-02T20:40:17Z

pandas/tests/indexing/multiindex/test_getitem.py

+    tm.assert_series_equal(result, expected)
+
+
+def test_getitem_duplicates_multiindex():


can you try to parameteize this on the input frame / expected output.

might make sense to split out the empty case to a new test

gfyoung · 2018-12-02T20:41:58Z

pandas/tests/indexing/multiindex/test_getitem.py

+        df['foo']['one'] = 2
+        return df
+
+    pytest.raises(com.SettingWithCopyError, f)


Any error message worth checking?

codecov · 2018-12-02T22:22:53Z

Codecov Report

Merging #24053 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #24053   +/-   ##
=======================================
  Coverage   42.45%   42.45%           
=======================================
  Files         161      161           
  Lines       51561    51561           
=======================================
  Hits        21888    21888           
  Misses      29673    29673

Flag	Coverage Δ
#single	`42.45% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/strings.py	`33% <0%> (ø)`	⬆️

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 92d25f0...47c4ce8. Read the comment docs.

codecov · 2018-12-02T22:22:53Z

Codecov Report

Merging #24053 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #24053   +/-   ##
=======================================
  Coverage   92.21%   92.21%           
=======================================
  Files         162      162           
  Lines       51761    51761           
=======================================
  Hits        47731    47731           
  Misses       4030     4030

Flag	Coverage Δ
#multiple	`90.61% <ø> (ø)`	⬆️
#single	`43% <ø> (ø)`	⬆️

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 5389987...3ff6159. Read the comment docs.

simonjayhawkins · 2018-12-02T22:23:52Z

pandas/tests/indexing/multiindex/test_getitem.py

+    tm.assert_series_equal(result, expected)
+
+    # xs
+    result = s.xs(0, level=0)


@jreback should this be result = s.xs(0, level=1)?

we could then paramatrize..

@pytest.mark.parametrize('access_method', [lambda s, x: s[:, x], lambda s, x: s.loc[:, x], lambda s, x: s.xs(x, level=1)]) @pytest.mark.parametrize('level1_value, expected', [ (0, Series([1], index=[0])), (1, Series([2, 3], index=[1, 2])) ]) def test_series_getitem_multiindex(access_method, level1_value, expected): # GH 6018 # series regression getitem with a multi-index s = Series([1, 2, 3]) s.index = MultiIndex.from_tuples([(0, 0), (1, 1), (2, 1)]) result = access_method(s, level1_value) tm.assert_series_equal(result, expected)

or is this OTT.

no, you certainly parameterize this

simonjayhawkins · 2018-12-11T11:00:00Z

@jreback @gfyoung

test_series_getitem_multiindex and test_getitem_duplicates_multiindex split and parametrized. error messages checked.

carry on and split and parametrize the remaining tests in this file within this PR or limit the diff?

jreback · 2018-12-11T13:54:46Z

thanks @simonjayhawkins keep em coming! I don't mind bigger ones, main issue is there are some other parts moving so easiest to get them in in self-contained units.

* upstream/master: pct change bug issue 21200 (pandas-dev#21235) DOC: Fix summaries not ending with a period (pandas-dev#24190) DOC: Add back currentmodule to api.rst (pandas-dev#24232) DOC: Fixed implicit imports for whatsnew (v >= version 20.0) (pandas-dev#24199) remove enum import for PY2 compat, xref pandas-dev#22802 (pandas-dev#24170) CI: Simplify Azure Pipelines configuration files (pandas-dev#23111) REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py (pandas-dev#24053)

…pandas-dev#24053)

simonjayhawkins added 2 commits December 2, 2018 20:19

remove test classes and replace catch_warnings

85037f7

Merge remote-tracking branch 'upstream/master' into mi-getitem

47c4ce8

gfyoung added Refactor Testing Indexing MultiIndex labels Dec 2, 2018

gfyoung reviewed Dec 2, 2018

View reviewed changes

jreback requested changes Dec 2, 2018

View reviewed changes

gfyoung reviewed Dec 2, 2018

View reviewed changes

simonjayhawkins commented Dec 2, 2018

View reviewed changes

simonjayhawkins added 6 commits December 10, 2018 12:03

resolve merge conflicts

d9ad11a

check error messages

6a50e17

split and parametrize test_series_getitem_multiindex

24d1a8d

split and parametrize test_getitem_duplicates_multiindex

933acef

Merge remote-tracking branch 'upstream/master' into mi-getitem

2dc4798

fix W605 invalid escape sequence

3ff6159

jreback added this to the 0.24.0 milestone Dec 11, 2018

jreback approved these changes Dec 11, 2018

View reviewed changes

jreback merged commit 9f29f88 into pandas-dev:master Dec 11, 2018

YuechengWu mentioned this pull request Dec 11, 2018

DOC: Fix summaries not ending with a period #24190

Merged

4 tasks

simonjayhawkins deleted the mi-getitem branch December 13, 2018 12:57

simonjayhawkins mentioned this pull request Dec 24, 2018

REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py #24409

Merged

2 tasks

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

REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py (…

498902b

…pandas-dev#24053)

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

REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py (…

d31a90c

…pandas-dev#24053)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py #24053

REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py #24053

simonjayhawkins commented Dec 2, 2018 •

edited

Loading

pep8speaks commented Dec 2, 2018 •

edited

Loading

gfyoung Dec 2, 2018

jreback Dec 2, 2018

jreback Dec 2, 2018

gfyoung Dec 2, 2018

codecov bot commented Dec 2, 2018

codecov bot commented Dec 2, 2018 •

edited

Loading

simonjayhawkins Dec 2, 2018

simonjayhawkins Dec 2, 2018

jreback Dec 3, 2018

simonjayhawkins commented Dec 11, 2018

jreback commented Dec 11, 2018

		tm.assert_series_equal(result, expected)


		def test_getitem_duplicates_multiindex():

REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py #24053

REF/TST: Add more pytest idiom to indexing/multiindex/test_getitem.py #24053

Conversation

simonjayhawkins commented Dec 2, 2018 • edited Loading

pep8speaks commented Dec 2, 2018 • edited Loading

Comment last updated on December 10, 2018 at 16:06 Hours UTC

gfyoung Dec 2, 2018

Choose a reason for hiding this comment

jreback Dec 2, 2018

Choose a reason for hiding this comment

jreback Dec 2, 2018

Choose a reason for hiding this comment

gfyoung Dec 2, 2018

Choose a reason for hiding this comment

codecov bot commented Dec 2, 2018

Codecov Report

codecov bot commented Dec 2, 2018 • edited Loading

Codecov Report

simonjayhawkins Dec 2, 2018

Choose a reason for hiding this comment

simonjayhawkins Dec 2, 2018

Choose a reason for hiding this comment

jreback Dec 3, 2018

Choose a reason for hiding this comment

simonjayhawkins commented Dec 11, 2018

jreback commented Dec 11, 2018

simonjayhawkins commented Dec 2, 2018 •

edited

Loading

pep8speaks commented Dec 2, 2018 •

edited

Loading

codecov bot commented Dec 2, 2018 •

edited

Loading