DOC: Improving docstring of take method #16948

matagus · 2017-07-15T15:49:16Z

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

pep8speaks · 2017-07-15T15:49:18Z

Hello @matagus! Thanks for updating the PR.

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

Comment last updated on August 21, 2017 at 08:25 Hours UTC

gfyoung · 2017-07-15T16:09:47Z

pandas/core/generic.py

@@ -1939,7 +1939,8 @@ def __delitem__(self, key):

    def take(self, indices, axis=0, convert=True, is_copy=True, **kwargs):
        """
-        Analogous to ndarray.take
+        Return an object formed from the elements in the given indices along an


We generally try to fit this description on one line.

jreback · 2017-07-15T15:51:59Z

pandas/core/generic.py

@@ -1948,6 +1949,44 @@ def take(self, indices, axis=0, convert=True, is_copy=True, **kwargs):
        convert : translate neg to pos indices (default)
        is_copy : mark the returned frame as a copy

+        Examples
+        --------
+        >>> import numpy as np


you don't need the imports here

jreback · 2017-07-15T15:52:42Z

pandas/core/generic.py

@@ -1939,7 +1939,8 @@ def __delitem__(self, key):

    def take(self, indices, axis=0, convert=True, is_copy=True, **kwargs):
        """
-        Analogous to ndarray.take
+        Return an object formed from the elements in the given indices along an


add that this is a positional selection.

jreback · 2017-07-15T15:53:01Z

pandas/core/generic.py

+             name   class  max_speed
+        3  monkey  mammal        NaN
+        2    lion  mammal       80.5
+
        Returns
        -------


add a See Also, showing ndarray.take

jreback · 2017-07-15T15:53:23Z

pandas/core/generic.py

+                              columns=('name', 'class', 'max_speed'))
+        >>> df
+             name   class  max_speed
+        0  falcon    bird      389.0


add a comment that these are positional selections (where you think it is needed)

jreback · 2017-07-15T15:53:57Z

pandas/core/generic.py

+        >>> import numpy as np
+        >>> import pandas as pd
+        >>> df = pd.DataFrame([('falcon', 'bird',    389.0),
+                               ('parrot', 'bird',     24.0),


give this an index (like [0, 3, 2, 1]) or something to emphasize that these are positional operations

codecov · 2017-07-15T16:25:11Z

Codecov Report

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

@@            Coverage Diff            @@
##             master   #16948   +/-   ##
=========================================
  Coverage          ?   91.01%           
=========================================
  Files             ?      162           
  Lines             ?    49567           
  Branches          ?        0           
=========================================
  Hits              ?    45114           
  Misses            ?     4453           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`88.79% <ø> (?)`
#single	`40.24% <ø> (?)`

Impacted Files	Coverage Δ
pandas/core/generic.py	`92.03% <ø> (ø)`

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 91245a7...0a07775. Read the comment docs.

jreback · 2017-07-19T10:25:40Z

can you update

jorisvandenbossche · 2017-08-07T12:32:23Z

@matagus Do you have time to update this according to the feedback?
(if not, no problem, but please indicate this so somebody else can take over)

jreback · 2017-08-18T00:56:51Z

@gfyoung if you have a chance can you fix up.

gfyoung · 2017-08-21T04:31:40Z

@jreback : Changes requested have been addressed. PTAL.

jorisvandenbossche

Looks good! Some nitpicks

jorisvandenbossche · 2017-08-21T07:55:09Z

pandas/core/generic.py

+        2  mammal       80.5
+        1  mammal        NaN
+
+        We may take elements using negative integers for positive indices.


can you clarify that this is for starting at the end?

jorisvandenbossche · 2017-08-21T07:57:04Z

pandas/core/generic.py

+        >>> df.take([-1, -2])
+             name   class  max_speed
+        1  monkey  mammal        NaN
+        2    lion  mammal       80.5


it is a bit unfortunate in the example that it is exactly indices [1, 2] for [-1, -2], so maybe change the indices a bit so they don't match? (also because you said "We may take elements using negative integers for positive indices.", which can be interpreted that negative values are interpreted as the positive index of that value)

jorisvandenbossche · 2017-08-21T07:57:40Z

pandas/core/generic.py

+
+        See Also
+        --------
+        ndarray.take


Does this need numpy. before it? (not sure, but for clarity maybe does no harm in any case)

jorisvandenbossche · 2017-08-21T07:59:16Z

pandas/core/generic.py

+
+        This means that we are not indexing according to actual values in
+        the index attribute of the object. We are indexing according to the
+        actual position of the element in the object's array of values.


I would just write "actual position of the elements in the object"
("object's array of values" can possibly cause some confusion I think)

jorisvandenbossche

Perfect!

jorisvandenbossche · 2017-08-21T08:28:28Z

Thanks @matagus and @gfyoung !

(btw, @gfyoung I personally find it easier that you add commits instead of amending the previous one, it's sometimes easier to see what has changes (in this case only small diff, so not really a problem, but for larger diffs can be helpful)

gfyoung · 2017-08-21T08:29:49Z

@jorisvandenbossche : Absolutely. Good to know.

On a separate note, what is the purpose of the convert parameter (I generally don't use it)? I see no difference in behavior AFAICT (using the docstring example):

>>> df.take([-1, -2])
             name   class  max_speed
        1  monkey  mammal        NaN
        3    lion  mammal       80.5
>>> df.take([-1, -2], convert=False)
             name   class  max_speed
        1  monkey  mammal        NaN
        3    lion  mammal       80.5

jorisvandenbossche · 2017-08-21T08:31:58Z

Good catch, reason is very simple, it is not passed through:

pandas/pandas/core/generic.py

Lines 2140 to 2142 in d0d28fe

    
           new_data = self._data.take(indices, 
        
                                      axis=self._get_block_manager_axis(axis), 
        
                                      convert=True, verify=True)

But not sure if this should actually be a public parameter

gfyoung · 2017-08-21T08:38:00Z

Doh! Yeah, that's a good point. Drilling further down reveals more strangeness:

pandas/pandas/core/internals.py

Lines 4096 to 4099 in d0d28fe

    
           if verify: 
        
               if ((indexer == -1) | (indexer >= n)).any(): 
        
                   raise Exception('Indices must be nonzero and less than ' 
        
                                   'the axis length')

The error message doesn't seem to match the check here IINM.

xref pandas-devgh-16948. The parameter is not respected.

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in 'take' implementations.

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

[ci skip] xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

xref gh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

jreback added the Docs label Jul 15, 2017

gfyoung reviewed Jul 15, 2017

View reviewed changes

jreback requested changes Jul 15, 2017

View reviewed changes

gfyoung added this to the 0.21.0 milestone Jul 19, 2017

gfyoung self-assigned this Aug 18, 2017

gfyoung force-pushed the docs-improving-dataframe-take-docstrings branch from 595cee8 to 9fb4aa4 Compare August 21, 2017 03:35

jorisvandenbossche reviewed Aug 21, 2017

View reviewed changes

matagus and others added 2 commits August 21, 2017 01:16

DOC: Improving docstring of take method

0d49def

DOC: Address reviewer comments

0a07775

gfyoung force-pushed the docs-improving-dataframe-take-docstrings branch 2 times, most recently from e8f7bc3 to 0a07775 Compare August 21, 2017 08:25

jorisvandenbossche approved these changes Aug 21, 2017

View reviewed changes

jorisvandenbossche merged commit d0d28fe into pandas-dev:master Aug 21, 2017

gfyoung removed their assignment Aug 21, 2017

gfyoung added a commit to forking-repos/pandas that referenced this pull request Aug 27, 2017

DEPR: Deprecate convert parameter in take

351cd69

xref pandas-devgh-16948. The parameter is not respected.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Aug 27, 2017

DEPR: Deprecate convert parameter in take

445fb25

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Aug 27, 2017

DEPR: Deprecate convert parameter in take

c9f42b2

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung mentioned this pull request Aug 27, 2017

DEPR: Deprecate convert parameter in take #17352

Merged

gfyoung added a commit to forking-repos/pandas that referenced this pull request Aug 28, 2017

DEPR: Deprecate convert parameter in take

9e0bcad

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Aug 28, 2017

DEPR: Deprecate convert parameter in take

4471360

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 23, 2017

DEPR: Deprecate convert parameter in take

35812d0

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 24, 2017

DEPR: Deprecate convert parameter in take

86b2a43

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 25, 2017

DEPR: Deprecate convert parameter in take

7d7f2c3

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 25, 2017

DEPR: Deprecate convert parameter in take

fd58e6a

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 25, 2017

DEPR: Deprecate convert parameter in take

7d5a299

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 26, 2017

DEPR: Deprecate convert parameter in take

5a27028

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 27, 2017

DEPR: Deprecate convert parameter in take

dfc58a1

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 27, 2017

DEPR: Deprecate convert parameter in take

81e1212

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 28, 2017

DEPR: Deprecate convert parameter in take

bb4a464

[ci skip] xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 29, 2017

DEPR: Deprecate convert parameter in take

cd30333

[ci skip] xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 29, 2017

DEPR: Deprecate convert parameter in take

d6a9864

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 29, 2017

DEPR: Deprecate convert parameter in take

dde1af0

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 29, 2017

DEPR: Deprecate convert parameter in take

136b93c

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 29, 2017

DEPR: Deprecate convert parameter in take

816fd5e

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 30, 2017

DEPR: Deprecate convert parameter in take

b3e2c4c

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 30, 2017

DEPR: Deprecate convert parameter in take

dbe8eb2

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 30, 2017

DEPR: Deprecate convert parameter in take

2603ce6

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 30, 2017

DEPR: Deprecate convert parameter in take

bae97d8

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 30, 2017

DEPR: Deprecate convert parameter in take

9b16edf

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 30, 2017

DEPR: Deprecate convert parameter in take

5f09002

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 30, 2017

DEPR: Deprecate convert parameter in take

9ff5e2e

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 30, 2017

DEPR: Deprecate convert parameter in take

a83784a

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Sep 30, 2017

DEPR: Deprecate convert parameter in take

249be32

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Oct 1, 2017

DEPR: Deprecate convert parameter in take

c16bc61

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Oct 1, 2017

DEPR: Deprecate convert parameter in take

ec299bc

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Oct 1, 2017

DEPR: Deprecate convert parameter in take

9325f21

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

jreback pushed a commit that referenced this pull request Oct 1, 2017

DEPR: Deprecate convert parameter in take (#17352)

458c1dc

xref gh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

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

DOC: Improving docstring of take method (pandas-dev#16948)

d293c4c

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

DEPR: Deprecate convert parameter in take (pandas-dev#17352)

0f12a7f

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

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

DEPR: Deprecate convert parameter in take (pandas-dev#17352)

5a998a8

xref pandas-devgh-16948. The parameter is not respected, nor is it a parameter in many 'take' implementations.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Improving docstring of take method #16948

DOC: Improving docstring of take method #16948

matagus commented Jul 15, 2017 •

edited

Loading

pep8speaks commented Jul 15, 2017 •

edited

Loading

gfyoung Jul 15, 2017

gfyoung Aug 21, 2017

jreback Jul 15, 2017

gfyoung Aug 21, 2017

jreback Jul 15, 2017

gfyoung Aug 21, 2017

jreback Jul 15, 2017

gfyoung Aug 21, 2017

jreback Jul 15, 2017

gfyoung Aug 21, 2017

jreback Jul 15, 2017

gfyoung Aug 21, 2017

codecov bot commented Jul 15, 2017 •

edited

Loading

jreback commented Jul 19, 2017

jorisvandenbossche commented Aug 7, 2017

jreback commented Aug 18, 2017

gfyoung commented Aug 21, 2017

jorisvandenbossche left a comment

jorisvandenbossche Aug 21, 2017

gfyoung Aug 21, 2017

jorisvandenbossche Aug 21, 2017

gfyoung Aug 21, 2017

jorisvandenbossche Aug 21, 2017

gfyoung Aug 21, 2017

jorisvandenbossche Aug 21, 2017

gfyoung Aug 21, 2017

jorisvandenbossche left a comment

jorisvandenbossche commented Aug 21, 2017

gfyoung commented Aug 21, 2017

jorisvandenbossche commented Aug 21, 2017

gfyoung commented Aug 21, 2017 •

edited

Loading

DOC: Improving docstring of take method #16948

DOC: Improving docstring of take method #16948

Conversation

matagus commented Jul 15, 2017 • edited Loading

pep8speaks commented Jul 15, 2017 • edited Loading

Comment last updated on August 21, 2017 at 08:25 Hours UTC

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 15, 2017 • edited Loading

Codecov Report

jreback commented Jul 19, 2017

jorisvandenbossche commented Aug 7, 2017

jreback commented Aug 18, 2017

gfyoung commented Aug 21, 2017

jorisvandenbossche left a 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

Choose a reason for hiding this comment

jorisvandenbossche left a comment

Choose a reason for hiding this comment

jorisvandenbossche commented Aug 21, 2017

gfyoung commented Aug 21, 2017

jorisvandenbossche commented Aug 21, 2017

gfyoung commented Aug 21, 2017 • edited Loading

matagus commented Jul 15, 2017 •

edited

Loading

pep8speaks commented Jul 15, 2017 •

edited

Loading

codecov bot commented Jul 15, 2017 •

edited

Loading

gfyoung commented Aug 21, 2017 •

edited

Loading