CLN: Comparison methods for MultiIndex should have consistent behaviour for all nlevels (GH21149) #21195

KalyanGokhale · 2018-05-24T18:27:12Z

closes (Assuming we don't want to disallow 1-level MultiIndexes), stop identifying MultiIndex by nlevels == 1 #21149
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Initial PR #21182 is closed - and now split in 2 different PRs

def cmp_method raised ValueError for equality / inequality comparisons of MultiIndex with nlevels == 1, which was inconsistent with behaviour for MultiIndex with nlevels > 1 (details of issue below) - this has now been fixed

Currently (as of 0.23.0), comparing MultiIndex of nlevels==1 with another of same length raises a ValueError e.g.

[In] midx=pd.MultiIndex.from_product([[0, 1]])
[In] midx
[Out] MultiIndex(levels=[[0, 1]],
           labels=[[0, 1]])
[In] midx == midx
[Out] ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

whereas the behaviour should be consistent with that for MultiIndex with nlevels>1 as follows:

[In] midx == midx
[Out] array([ True,  True])

Updating to 0.23.0

Update 18 May

22May

24 May fork update

GH21149-1

Removed the following test, which was causing builds to fail (?). This was working when tested on my command line (Mac OS Terminal) # Greater-than test: non-MultiIndex Index object vs MultiIndex object with tm.assert_raises_regex(TypeError, 'not supported'): midx > idx

pep8speaks · 2018-05-24T19:18:28Z

Hello @KalyanGokhale! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on June 14, 2018 at 10:23 Hours UTC

codecov · 2018-05-24T19:18:48Z

Codecov Report

❗ No coverage uploaded for pull request base (master@fd121ed). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #21195   +/-   ##
=========================================
  Coverage          ?    91.9%           
=========================================
  Files             ?      153           
  Lines             ?    49607           
  Branches          ?        0           
=========================================
  Hits              ?    45590           
  Misses            ?     4017           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.3% <100%> (?)`
#single	`41.89% <100%> (?)`

Impacted Files	Coverage Δ
pandas/core/indexes/base.py	`96.62% <100%> (ø)`

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 fd121ed...73cac75. Read the comment docs.

jreback · 2018-05-24T22:01:40Z

doc/source/whatsnew/v0.23.1.txt

@@ -78,7 +78,8 @@ Indexing

 - Bug in :meth:`Series.reset_index` where appropriate error was not raised with an invalid level name (:issue:`20925`)
 - Bug in :func:`interval_range` when ``start``/``periods`` or ``end``/``periods`` are specified with float ``start`` or ``end`` (:issue:`21161`)
-
+- Bug in comparison operations for :class:`MultiIndex` where error was raised on equality / inequality comparison involving a MultiIndex with self.nlevels == 1 (:issue:`21149`)
+- 


double backticks on MultiIndex

jreback · 2018-05-24T22:02:05Z

pandas/tests/indexes/test_multi.py

+        (pd.MultiIndex.from_product([[0, 1]]), pd.Series(range(2)), 2)])
+    def test_multiindex_compare(self, midx, idx, count):
+        # GH 21149
+        '''Ensure comparison operations for MultiIndex with nlevels == 1


use triple-double quotes

jreback · 2018-05-24T22:02:20Z

pandas/tests/indexes/test_multi.py

+        '''
+        expected = pd.Series([True]).repeat(count)
+        expected.reset_index(drop=True, inplace=True)
+        # Equality self-test: MultiIndex object vs self


blank line between cases

jreback · 2018-05-24T22:02:51Z

pandas/tests/indexes/test_multi.py

+            behave consistently with those for MultiIndex with nlevels > 1
+        '''
+        expected = pd.Series([True]).repeat(count)
+        expected.reset_index(drop=True, inplace=True)


don't use inplace in tests

@jreback Thanks - here inplace is being used to only create the expected outcome for testing. inplace is not being used in the test assert_series_equal
OK to keep then?

done - I have simplified the tests.
now only kept the required cases - as such have removed the parametrisation and also use of inplace in creating the expected results

jreback · 2018-05-24T22:04:38Z

pandas/tests/indexes/test_multi.py

+        result = pd.Series(midx == midx)
+        tm.assert_series_equal(result, expected)
+        # Equality self-test: non-MultiIndex Index object vs self
+        result = (idx == idx)


where did the idea for these test come from? what exactly are you trying to test here?

These tests are trying to ensure the behaviour on comparison between MultiIndex vs MultiIndex, MultiIndex vs Index is consistent, irrespective of nlevels. Also, ensuring that the comparison behaviour for Index vs Index has not changed due to this PR.

Currently (as of 0.23.0), comparing MultiIndex of nlevels==1 with another of same length raises a ValueError e.g.

[In] midx=pd.MultiIndex.from_product([[0, 1]]) [In] midx [Out] MultiIndex(levels=[[0, 1]], labels=[[0, 1]]) [In] midx == midx [Out] ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

whereas the behaviour should be consistent with that for MultiIndex with nlevels>1 as follows:

[In] midx == midx [Out] array([ True, True])

in this test, probably should be OK to only keep the midx == midx and midx > midx checks

toobaz · 2018-05-25T08:16:04Z

def cmp_method raised ValueError for equality / inequality comparisons of MultiIndex with nlevels == 1

Good catch!

KalyanGokhale · 2018-05-31T12:12:06Z

@jreback @toobaz are any other edits needed on this? Thanks

GH21149-1a

Rebased and updated whatsnew v0.23.2

KalyanGokhale · 2018-06-14T01:33:22Z

Any other work needed on this PR? Thanks

jreback · 2018-06-14T10:23:37Z

thanks, if anything residual is remaining from #21149 let's open a new issue

…ur for all nlevels (GH21149) (pandas-dev#21195)

…ur for all nlevels (GH21149) (#21195) (cherry picked from commit a8738ba)

…ur for all nlevels (GH21149) (pandas-dev#21195)

KalyanGokhale added 6 commits May 17, 2018 22:55

Merge pull request #1 from pandas-dev/master

d0c7ebc

Updating to 0.23.0

Merge pull request #3 from pandas-dev/master

143566a

Update 18 May

Merge pull request #4 from pandas-dev/master

dd60b4e

22May

Merge remote-tracking branch 'upstream/master'

d4d2db3

24 May fork update

Initial commit

370d509

GH21149-1

Fixed PEP8 formatting issues

b661ca1

GH21149-1

KalyanGokhale changed the title ~~BUG: Comparison methods for MultiIndex with nlevels == 1 should have consistent behavior (GH21149)~~ BUG: Comparison methods for MultiIndex with nlevels == 1 should have consistent behaviour with MultiIndex having nlevels > 1 (GH21149) May 24, 2018

KalyanGokhale mentioned this pull request May 24, 2018

BUG: Should not raises errors in .set_names and comparison methods for MultiIndex with nlevels == 1 (GH21149) #21182

Closed

4 tasks

KalyanGokhale changed the title ~~BUG: Comparison methods for MultiIndex with nlevels == 1 should have consistent behaviour with MultiIndex having nlevels > 1 (GH21149)~~ BUG: Comparison methods for MultiIndex should have consistent behaviour for all nlevels (GH21149) May 24, 2018

Update test_multi.py

d27736d

jreback requested changes May 24, 2018

View reviewed changes

jreback added MultiIndex Clean labels May 24, 2018

KalyanGokhale added 2 commits May 27, 2018 14:54

Update v0.23.1.txt

f90cf94

Simplified tests

6ad6e7e

KalyanGokhale changed the title ~~BUG: Comparison methods for MultiIndex should have consistent behaviour for all nlevels (GH21149)~~ CLN: Comparison methods for MultiIndex should have consistent behaviour for all nlevels (GH21149) Jun 5, 2018

KalyanGokhale added 5 commits June 13, 2018 22:16

Update v0.23.1.txt

a2cd674

Removed changes to whatsnew v0.23.1

bf4494f

Removed changes from whatsnew v0.23.1

6925732

Updated whatsnew v0.23.2

a27cc98

GH21149-1a

Merge pull request #7 from KalyanGokhale/GH21149-1a

b504276

Rebased and updated whatsnew v0.23.2

jreback added this to the 0.23.2 milestone Jun 14, 2018

jreback added 2 commits June 14, 2018 06:21

Merge branch 'master' into PR_TOOL_MERGE_PR_21195

f0723e7

doc

73cac75

jreback approved these changes Jun 14, 2018

View reviewed changes

jreback merged commit a8738ba into pandas-dev:master Jun 14, 2018

jreback added the Needs Backport label Jun 14, 2018

KalyanGokhale deleted the GH21149-1 branch June 14, 2018 11:39

david-liu-brattle-1 pushed a commit to david-liu-brattle-1/pandas that referenced this pull request Jun 18, 2018

CLN: Comparison methods for MultiIndex should have consistent behavio…

37e652d

…ur for all nlevels (GH21149) (pandas-dev#21195)

jorisvandenbossche removed the Needs Backport label Jun 29, 2018

jorisvandenbossche pushed a commit that referenced this pull request Jun 29, 2018

CLN: Comparison methods for MultiIndex should have consistent behavio…

787ef30

…ur for all nlevels (GH21149) (#21195) (cherry picked from commit a8738ba)

jorisvandenbossche pushed a commit that referenced this pull request Jul 2, 2018

CLN: Comparison methods for MultiIndex should have consistent behavio…

2272ef4

…ur for all nlevels (GH21149) (#21195) (cherry picked from commit a8738ba)

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

CLN: Comparison methods for MultiIndex should have consistent behavio…

52eb5e7

…ur for all nlevels (GH21149) (pandas-dev#21195)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLN: Comparison methods for MultiIndex should have consistent behaviour for all nlevels (GH21149) #21195

CLN: Comparison methods for MultiIndex should have consistent behaviour for all nlevels (GH21149) #21195

KalyanGokhale commented May 24, 2018 •

edited by jreback

Loading

pep8speaks commented May 24, 2018 •

edited

Loading

codecov bot commented May 24, 2018 •

edited

Loading

jreback May 24, 2018

jreback May 24, 2018

KalyanGokhale May 27, 2018

jreback May 24, 2018

KalyanGokhale May 27, 2018

jreback May 24, 2018

KalyanGokhale May 25, 2018

KalyanGokhale May 27, 2018

jreback May 24, 2018

KalyanGokhale May 25, 2018 •

edited

Loading

KalyanGokhale May 26, 2018

toobaz commented May 25, 2018

KalyanGokhale commented May 31, 2018

KalyanGokhale commented Jun 14, 2018

jreback commented Jun 14, 2018

CLN: Comparison methods for MultiIndex should have consistent behaviour for all nlevels (GH21149) #21195

CLN: Comparison methods for MultiIndex should have consistent behaviour for all nlevels (GH21149) #21195

Conversation

KalyanGokhale commented May 24, 2018 • edited by jreback Loading

pep8speaks commented May 24, 2018 • edited Loading

Comment last updated on June 14, 2018 at 10:23 Hours UTC

codecov bot commented May 24, 2018 • edited Loading

Codecov Report

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

KalyanGokhale May 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toobaz commented May 25, 2018

KalyanGokhale commented May 31, 2018

KalyanGokhale commented Jun 14, 2018

jreback commented Jun 14, 2018

KalyanGokhale commented May 24, 2018 •

edited by jreback

Loading

pep8speaks commented May 24, 2018 •

edited

Loading

codecov bot commented May 24, 2018 •

edited

Loading

KalyanGokhale May 25, 2018 •

edited

Loading