DOC/TST: Add examples to MultiIndex.get_level_values + related changes #17414

topper-123 · 2017-09-01T17:55:32Z

closes #xxxx
[x ] tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

I've added examples to MultiIndex.get_level_values. Also I've done some related changes:

made return value of Index._get_level_values
Added test for when supplying name to Index._get_level_values. The method could always take level name, but the doc said only integer could be supplied.

pep8speaks · 2017-09-01T17:55:37Z

Hello @topper-123! Thanks for updating the PR.

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

Comment last updated on September 02, 2017 at 12:40 Hours UTC

codecov · 2017-09-01T19:28:36Z

Codecov Report

Merging #17414 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #17414      +/-   ##
==========================================
- Coverage   91.15%   91.14%   -0.02%     
==========================================
  Files         163      163              
  Lines       49581    49581              
==========================================
- Hits        45198    45190       -8     
- Misses       4383     4391       +8

Flag	Coverage Δ
#multiple	`88.92% <ø> (ø)`	⬆️
#single	`40.25% <ø> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/base.py	`96.29% <ø> (ø)`	⬆️
pandas/core/indexes/multi.py	`96.9% <ø> (ø)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.72% <0%> (-0.1%)`	⬇️
pandas/core/indexes/datetimes.py	`95.43% <0%> (+0.09%)`	⬆️

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 1981b67...cc67b9a. Read the comment docs.

gfyoung · 2017-09-02T07:19:52Z

pandas/core/indexes/base.py


        Parameters
        ----------
-        level : int
+        level : int or level name


The general format is to specify the data type on the first line followed by an explanation of their semantics in a subsequent block.

Ok, I've changed this, also for MultiIndex.

gfyoung · 2017-09-02T07:20:37Z

pandas/core/indexes/base.py

@@ -2529,15 +2529,20 @@ def set_value(self, arr, key, value):
    def _get_level_values(self, level):
        """
        Return an Index of values for requested level, equal to the length
-        of the index
+        of the index.


Not Blocking : I would really like if we could condense this into one line (doc-string convention), but it's hard to see ATM what to remove from this...

I haven't got a better proposal either.

gfyoung · 2017-09-02T07:21:19Z

pandas/core/indexes/base.py


        Returns
        -------
-        values : Index
+        self : Index


You aren't returning self are you? I would leave this line alone and instead add a line below explaining what the output is.

Actually it is returning self. this method probably meant to make the API the same for MultiIndex and other Index types, so for Index types this really doesn't do much. The name values is used for MultiIndex, but isn't used here...

should I still change it?

@topper-123 : Sorry, I wasn't able to check the actual file on my phone. Yes, you are correct that self is returned. While I do stand corrected, I do think leaving it unchanged is preferable for consistency. You can however, in a line beneath that explain that values in this implementation is just self because there is only one level.

gfyoung · 2017-09-02T07:21:46Z

pandas/tests/indexes/test_base.py

@@ -1438,6 +1438,12 @@ def test_get_level_values(self):
        result = self.strIndex.get_level_values(0)
        tm.assert_index_equal(result, self.strIndex)

+        # test with name


Reference your PR here.

topper-123 · 2017-09-02T08:52:09Z

pandas/core/indexes/multi.py


        Returns
        -------
        values : Index
+
+        Examples


Note that I've made the examples a bit simpler. No need to demonstrate CategoricalIndex etc. here...

gfyoung · 2017-09-02T08:52:56Z

pandas/core/indexes/multi.py

-        level : int or level name
+        level : int or str
+            ``level`` is either the integer position of the level in the
+            MultiIndex, or the name of the level.

        Returns
        -------
        values : Index


Let's explain what values is here since you are updating the docs.

gfyoung · 2017-09-02T08:53:47Z

pandas/core/indexes/base.py


        Returns
        -------
        values : Index
+            Because there is only one level when the index has one level,
+            the return value is always self in this case. 


I would simplify to just:

"self, as there is only one level in the Index"

Yes, thats better

gfyoung · 2017-09-02T08:54:30Z

pandas/tests/indexes/test_base.py

@@ -1438,6 +1438,12 @@ def test_get_level_values(self):
        result = self.strIndex.get_level_values(0)
        tm.assert_index_equal(result, self.strIndex)

+        # GH 17414


You didn't need to necessarily remove your other note. I was suggesting that you just add a reference to the issue somewhere in the comments.

jreback · 2017-09-06T12:29:32Z

lgtm. @gfyoung unless you had any more comments, merge away.

gfyoung · 2017-09-06T14:55:20Z

Thanks @topper-123 !

pandas-dev#17414)

Add examples to MultiIndex.get_level_values + related changes

630b664

topper-123 force-pushed the add_examples branch from 61adbe9 to 630b664 Compare September 1, 2017 17:57

gfyoung added Docs Testing pandas testing functions or related to the test suite labels Sep 2, 2017

gfyoung reviewed Sep 2, 2017

View reviewed changes

gfyoung added the MultiIndex label Sep 2, 2017

topper-123 force-pushed the add_examples branch from e9aa20b to 01f792b Compare September 2, 2017 08:42

Updated according to comments (GH 17414)

8bfeec3

topper-123 force-pushed the add_examples branch from 01f792b to 8bfeec3 Compare September 2, 2017 08:50

topper-123 commented Sep 2, 2017

View reviewed changes

gfyoung reviewed Sep 2, 2017

View reviewed changes

topper-123 force-pushed the add_examples branch from 1f3b7ed to e62dc50 Compare September 2, 2017 09:04

added explanation for return value of MultiIndex.get_level_values

cc67b9a

topper-123 force-pushed the add_examples branch from e62dc50 to cc67b9a Compare September 2, 2017 12:40

jreback added this to the 0.21.0 milestone Sep 6, 2017

gfyoung merged commit d457791 into pandas-dev:master Sep 6, 2017

jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Sep 10, 2017

DOC/TST: Add examples to MultiIndex.get_level_values + related changes (

708e4e6

pandas-dev#17414)

topper-123 deleted the add_examples branch September 11, 2017 21:17

jowens pushed a commit to jowens/pandas that referenced this pull request Sep 20, 2017

DOC/TST: Add examples to MultiIndex.get_level_values + related changes (

047727a

pandas-dev#17414)

alanbato pushed a commit to alanbato/pandas that referenced this pull request Nov 10, 2017

DOC/TST: Add examples to MultiIndex.get_level_values + related changes (

feb5c92

pandas-dev#17414)

No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017

DOC/TST: Add examples to MultiIndex.get_level_values + related changes (

d8e01c4

pandas-dev#17414)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC/TST: Add examples to MultiIndex.get_level_values + related changes #17414

DOC/TST: Add examples to MultiIndex.get_level_values + related changes #17414

topper-123 commented Sep 1, 2017

pep8speaks commented Sep 1, 2017 •

edited

Loading

codecov bot commented Sep 1, 2017 •

edited

Loading

gfyoung Sep 2, 2017 •

edited

Loading

topper-123 Sep 2, 2017

gfyoung Sep 2, 2017

topper-123 Sep 2, 2017 •

edited

Loading

gfyoung Sep 2, 2017

topper-123 Sep 2, 2017

gfyoung Sep 2, 2017

gfyoung Sep 2, 2017

topper-123 Sep 2, 2017

gfyoung Sep 2, 2017

gfyoung Sep 2, 2017

topper-123 Sep 2, 2017

gfyoung Sep 2, 2017 •

edited

Loading

jreback commented Sep 6, 2017

gfyoung commented Sep 6, 2017

DOC/TST: Add examples to MultiIndex.get_level_values + related changes #17414

DOC/TST: Add examples to MultiIndex.get_level_values + related changes #17414

Conversation

topper-123 commented Sep 1, 2017

pep8speaks commented Sep 1, 2017 • edited Loading

Comment last updated on September 02, 2017 at 12:40 Hours UTC

codecov bot commented Sep 1, 2017 • edited Loading

Codecov Report

gfyoung Sep 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topper-123 Sep 2, 2017 • edited Loading

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

gfyoung Sep 2, 2017 • edited Loading

Choose a reason for hiding this comment

jreback commented Sep 6, 2017

gfyoung commented Sep 6, 2017

pep8speaks commented Sep 1, 2017 •

edited

Loading

codecov bot commented Sep 1, 2017 •

edited

Loading

gfyoung Sep 2, 2017 •

edited

Loading

topper-123 Sep 2, 2017 •

edited

Loading

gfyoung Sep 2, 2017 •

edited

Loading