DOC: update and seperate the Series.drop and Dataframe.drop docstring #20120

JennaVergeynst · 2018-03-10T12:48:40Z

Checklist for the pandas documentation sprint (ignore this if you are doing
an unrelated PR):

PR title is "DOC: update the docstring"
The validation script passes: scripts/validate_docstrings.py <your-function-or-method>
The PEP8 style check passes: git diff upstream/master -u -- "*.py" | flake8 --diff
The html version looks good: python doc/make.py --single <your-function-or-method>
It has been proofread on language by another sprint participant

Please include the output of the validation script below between the "```" ticks:

Errors found:
Errors in parameters section
Parameter "columns" description should finish with "."

This error is caused by .. versionadded:: 0.21.0, which needs to stay in place.

pep8speaks · 2018-03-10T12:48:45Z

Hello @JennaVergeynst! Thanks for updating the PR.

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

Comment last updated on March 12, 2018 at 21:59 Hours UTC

stijnvanhoey · 2018-03-10T14:32:52Z

pandas/core/frame.py

+                weight 	1.0 	0.8
+        """
+        return super(DataFrame,
+                     self).drop(labels=labels, axis=axis, index=index,


indendation is a bit strange here. I would set the self on the same line

stijnvanhoey · 2018-03-10T14:37:03Z

pandas/core/frame.py

+            Whether to drop labels from the index (0 / 'index') or
+            columns (1 / 'columns').
+        index : None
+            Redundant for application on Series, but index can be used instead


As this is now in the data.frame section, this is probably not redundant?

stijnvanhoey · 2018-03-10T14:37:36Z

pandas/core/frame.py

+            Redundant for application on Series, but index can be used instead
+            of labels.
+        columns : None
+            Redundant for application on Series, but index can be used instead


As this is now in the data.frame section, this is probably not redundant?

stijnvanhoey · 2018-03-10T14:39:21Z

pandas/core/frame.py

+
+            .. versionadded:: 0.21.0
+        level : int or level name, optional
+            For MultiIndex.


Is it useful to make this more detailed? e.g. Level for which the labels will be removed

stijnvanhoey · 2018-03-10T14:40:31Z

pandas/core/frame.py

+
+        Returns
+        -------
+        dropped : type of caller


In the generic, this was the same as the caller, but here we know it is a dataframe

stijnvanhoey · 2018-03-10T14:41:24Z

pandas/core/frame.py

+                weight 	1.0 	0.8
+                length 	0.3 	0.2
+
+        >>> df.drop(index='cow',columns='small')


missing space after ,

stijnvanhoey · 2018-03-10T14:41:43Z

pandas/core/frame.py

+
+        Drop columns and/or rows of MultiIndex
+
+        >>> midx = pd.MultiIndex(levels=[['lama','cow','falcon'],


missing spaces after ,

stijnvanhoey · 2018-03-10T14:42:17Z

pandas/core/series.py

+
+        Returns
+        -------
+        dropped : type of caller


pandas.Series

stijnvanhoey · 2018-03-10T14:59:03Z

pandas/core/frame.py

+            Index or column labels to drop.
+        axis : int or axis name, default 0
+            Whether to drop labels from the index (0 / 'index') or
+            columns (1 / 'columns').


I would write (0 or 'index') and columns (1 or 'columns')

stijnvanhoey · 2018-03-10T15:04:52Z

pandas/core/frame.py

+        ----------
+        labels : single label or list-like
+            Index or column labels to drop.
+        axis : int or axis name, default 0


cfr. gitter discussion: axis : {0 or 'index', 1 or 'columns'}, default 0

jreback · 2018-03-10T15:02:49Z

pandas/core/frame.py

+        DataFrame.dropna : Return DataFrame with labels on given axis omitted
+            where (all or any) data are missing
+        DataFrame.drop_duplicates : Return DataFrame with duplicate rows
+            removed, optionally only considering certain columns


reference DataFrame.loc (first) as well as this is an indexing operations

also Series.drop

jreback · 2018-03-10T15:04:00Z

pandas/core/frame.py

+           A  B   C   D
+        2  8  9  10  11
+
+        Drop columns and/or rows of MultiIndex


MultiIndexed DataFrame

jreback · 2018-03-10T15:04:56Z

pandas/core/frame.py

+                                    [0.3,0.2]])
+        >>> df
+                        big 	small
+        lama 	speed 	45.0 	30.0


I don't think this is layed out correctly, a copy-paste issue?

jreback · 2018-03-10T15:06:15Z

pandas/core/frame.py

+        >>> midx = pd.MultiIndex(levels=[['lama','cow','falcon'],
+        ...                              ['speed','weight','length']],
+        ...                      labels=[[0, 0, 0, 1, 1, 1, 2, 2, 2],
+        ...                              [0, 1, 2, 0, 1, 2, 0, 1, 2]])


I would make this example a bit simpler, maybe using integers for the value column (and order them, e.g. use np.arange(len(midx)) to create (big/small)

jreback · 2018-03-10T15:06:45Z

pandas/core/frame.py

+        DataFrame.dropna : Return DataFrame with labels on given axis omitted
+            where (all or any) data are missing
+        DataFrame.drop_duplicates : Return DataFrame with duplicate rows
+            removed, optionally only considering certain columns


also Series.drop

codecov · 2018-03-12T21:46:58Z

Codecov Report

Merging #20120 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #20120      +/-   ##
==========================================
- Coverage   91.72%    91.7%   -0.03%     
==========================================
  Files         150      150              
  Lines       49156    49156              
==========================================
- Hits        45090    45078      -12     
- Misses       4066     4078      +12

Flag	Coverage Δ
#multiple	`90.08% <100%> (-0.03%)`	⬇️
#single	`41.85% <54.54%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/generic.py	`95.84% <ø> (ø)`	⬆️
pandas/core/frame.py	`97.18% <100%> (ø)`	⬆️
pandas/core/series.py	`93.86% <100%> (ø)`	⬆️
pandas/plotting/_converter.py	`65.07% <0%> (-1.74%)`	⬇️
pandas/core/base.py	`96.78% <0%> (-0.02%)`	⬇️
pandas/core/indexes/base.py	`96.66% <0%> (-0.01%)`	⬇️
pandas/core/indexes/multi.py	`95.06% <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 840d432...643db5c. Read the comment docs.

jorisvandenbossche · 2018-03-13T16:06:06Z

@JennaVergeynst Thanks for the PR!

DOC: seperate drop doc for series and dataframes

3b5d101

update pep errors

8c5d0a6

stijnvanhoey suggested changes Mar 10, 2018

View reviewed changes

stijnvanhoey reviewed Mar 10, 2018

View reviewed changes

jreback requested changes Mar 10, 2018

View reviewed changes

jreback added Docs Indexing Related to indexing on series/frames, not to indexes themselves Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Mar 10, 2018

JennaVergeynst added 3 commits March 10, 2018 16:35

update drop docstring

1268785

update drop docstring

6ade1b7

update DOC

33b62a6

jorisvandenbossche mentioned this pull request Mar 10, 2018

DOC: update the Seriex.idxmax and Series.idxmin docstring #20174

Merged

5 tasks

Update frame.py

79779a4

jorisvandenbossche added 2 commits March 12, 2018 22:48

Update series.py

029560b

fix whitespace

643db5c

jorisvandenbossche approved these changes Mar 12, 2018

View reviewed changes

jorisvandenbossche added this to the 0.23.0 milestone Mar 13, 2018

jorisvandenbossche merged commit c6eec25 into pandas-dev:master Mar 13, 2018

JennaVergeynst deleted the DOC_series_drop branch March 13, 2018 17:49


		Drop columns and/or rows of MultiIndex

		>>> midx = pd.MultiIndex(levels=[['lama','cow','falcon'],

Uh oh!

DOC: update and seperate the Series.drop and Dataframe.drop docstring #20120

DOC: update and seperate the Series.drop and Dataframe.drop docstring #20120

Uh oh!

Conversation

JennaVergeynst commented Mar 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pep8speaks commented Mar 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated on March 12, 2018 at 21:59 Hours UTC

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jorisvandenbossche commented Mar 13, 2018

Uh oh!

Uh oh!

JennaVergeynst commented Mar 10, 2018 •

edited

Loading

pep8speaks commented Mar 10, 2018 •

edited

Loading

codecov bot commented Mar 12, 2018 •

edited

Loading