DOC: update the pandas.Series.str.strip docstring #20863

Cheukting · 2018-04-29T11:24:12Z

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

WillAyd

Thanks for the PR. Can you post the results of the doc string validator?

WillAyd · 2018-04-30T03:18:07Z

pandas/core/strings.py

+
+    Parameters
+    ----------
+    to_strip : str


to_strip : str, optional

WillAyd · 2018-04-30T03:19:47Z

pandas/core/strings.py

+    Parameters
+    ----------
+    to_strip : str
+        Specifying the set of characters to be removed. 


Let's just simplify this and say "The set of characters to be removed". It matches the standard Python string method so no need to be too wordy

WillAyd · 2018-04-30T03:20:27Z

pandas/core/strings.py

+
+    >>> s = pd.Series([' ant', 'bee ', ' cat  '])
+
+    >>> s


This is simple enough that you don't need to print the Series as is

WillAyd · 2018-04-30T03:21:29Z

pandas/core/strings.py

+
+    Examples
+    --------
+    Striping whitespaces for Series


"Striping" is a typo here and below, though these examples are simple enough that I don't think you need to explain them

WillAyd · 2018-04-30T03:23:32Z

pandas/core/strings.py

+
+    Striping a set of characters for Index
+
+    >>> df = pd.DataFrame(index=['1.ant ','2._bee__','3. cat_'])


Just create the index directly instead of creating a DataFrame

index = pd.Index(['1.ant ']) ...

WillAyd · 2018-04-30T03:23:43Z

pandas/core/strings.py

+
+    >>> df = pd.DataFrame(index=['1.ant ','2._bee__','3. cat_'])
+
+    >>> df.index


Again simple enough you don't need to print

WillAyd · 2018-04-30T03:24:27Z

pandas/core/strings.py

+    2    cat
+    dtype: object
+
+    Striping a set of characters for Index


Typo here, but as mentioned above these are basic enough that I personally don't think you need to explain in detail

pep8speaks · 2018-04-30T07:46:50Z

Hello @Cheukting! Thanks for updating the PR.

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

Comment last updated on July 07, 2018 at 20:05 Hours UTC

codecov · 2018-04-30T07:47:12Z

Codecov Report

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

@@            Coverage Diff            @@
##             master   #20863   +/-   ##
=========================================
  Coverage          ?    91.8%           
=========================================
  Files             ?      153           
  Lines             ?    49411           
  Branches          ?        0           
=========================================
  Hits              ?    45361           
  Misses            ?     4050           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.19% <ø> (?)`
#single	`41.91% <ø> (?)`

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

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 dcbf8b5...0d6a4cc. Read the comment docs.

Cheukting · 2018-04-30T07:54:23Z

###############################################################################

################################## Validation #################################

###############################################################################

Docstring for "pandas.Series.str.strip" correct. :)

datapythonista

Added some comments.

Also, #20628 was working on this too. I think your changes look better, but you can take a look at that PR and see if there is any idea that you can copy here.

datapythonista · 2018-04-30T19:34:36Z

pandas/core/strings.py

+
+    See Also
+    --------
+    str.slice : Slice substrings from each element in the Series/Index


slice doesn't seem very related to strip.

Not sure if there is anything else related, but we should have here Series.str.strip, Series.str.lstrip and Series.str.rstrip, so they link each other.

datapythonista · 2018-04-30T19:35:17Z

pandas/core/strings.py

+    to_strip : str, optional
+        Specifying the set of characters to be removed.
+        All combinations of this set of characters will be stripped.
+        Default value is None, which means whaitspaces will be removed.

    Returns
    -------
    stripped : Series/Index of objects


Can you get rid of the name stripped, just leave the type in that line, and in the next add a description of what is being returned?

datapythonista · 2018-04-30T19:40:17Z

pandas/core/strings.py

+
+    Parameters
+    ----------
+    to_strip : str, optional


this is not optional, the default is Ǹone which means whitespaces are stripped You can add to_strip : str or None, default None (whitespaces are removed). And take it out from the description.

datapythonista · 2018-04-30T19:40:55Z

pandas/core/strings.py

+    Stripping whitespaces for Series
+
+    >>> s = pd.Series([' ant', 'bee ', ' cat  '])
+


this blank line is unnecessary.

datapythonista · 2018-04-30T19:47:42Z

pandas/core/strings.py

+    Index(['1.ant ', '2._bee__', '3. cat_'], dtype='object')
+
+    >>> idx.str.strip('123._ ')
+    Index(['ant', 'bee', 'cat'], dtype='object')


Can we use a single example, with a single Series with multiple values that illustrate:

The method without parameters s.str.strip() and how it strips not only spaces but new lines and tabs

The characters to strip in the left, the right and both sides

The use of the 3 methods being documented lstrip, rstrip, rstrip

The use of another character to strip (something that looks like a real-world example)

Stripping more than a single character (also something a bit more real-world if possible would be great.

datapythonista · 2018-05-01T09:32:05Z

pandas/core/strings.py

-    Stripping a set of characters for Index
-
-    >>> idx = pd.Index(['1.ant ','2._bee__','3. cat_'])
+    >>> s.str.strip('123.!? \n\t')


Personally I find this example overcomplicated. I think something more realistic and simple could be:

Illustrate the default to_strip parameter (whitespaces) with rstrip in a value like ` text \n'

lstrip(to_strip='0') for a value like 0010010)

And then you can show strip(to_strip=' \n0') with both values

I still wants to keep the 'Animal' theme in the example but I will simplify the lstrip and rstrip example

alsobay · 2018-05-01T15:48:32Z

Hi @Cheukting, great work! As @datapythonista pointed out in #20897, it seems that our wires crossed during the sprint and we ended up working on the same thing.

Seeing as this PR got in earlier, please feel free to grab anything from our PR that might help address Marc's notes, and we'll close ours. If you don't plan to refine this PR, please let us know and we'll continue on #20897.

Cheukting · 2018-05-01T15:54:11Z

Thanks @alsobay and @jalammar! I really like your PR and I think I may borrow some ideas and put it here. Feel free to keep commenting and reviewing this PR as it is our collaboration effort :-)

jreback · 2018-05-04T10:42:34Z

pandas/core/strings.py

+
+    Parameters
+    ----------
+    to_strip : str or None, default None (whitespaces are removed)


the coment about whitespaces are removed should be on the next line with a case, e.g.
If None then whitespaces are removed

jreback · 2018-05-04T10:43:07Z

pandas/core/strings.py

+
+    Examples
+    --------
+    >>> s = pd.Series(['1. Ant.  ', '2. Bee!\n', '3. Cat?\t'])


can you add a np.nan in the example (in place of an entry or in addition is ok)

mroeschke · 2018-07-07T20:07:48Z

Looks great! Will merge on green.

jreback · 2018-07-07T22:09:24Z

@mroeschke merge away

mroeschke · 2018-07-07T22:12:37Z

Thanks @Cheukting!

* DOC: update the pandas.Series.str.strip docstring * DOC: update the pandas.Series.str.strip docstring * typo fixing and improve Index example * Fix PEP8 * Improve parameter explaination * Improve documentation, adding lstrip and rstrip example * simplify lstrip and rstrip * added np.nan example

Cheukting added 2 commits April 29, 2018 11:58

DOC: update the pandas.Series.str.strip docstring

a54a326

DOC: update the pandas.Series.str.strip docstring

fee3c61

WillAyd requested changes Apr 30, 2018

View reviewed changes

typo fixing and improve Index example

8b36512

Cheukting added 2 commits April 30, 2018 08:50

Fix PEP8

660b4fc

Improve parameter explaination

e6fe921

datapythonista mentioned this pull request Apr 30, 2018

DOC: update the pandas.Series.str.strip docstring #20628

Closed

5 tasks

datapythonista reviewed Apr 30, 2018

View reviewed changes

Improve documentation, adding lstrip and rstrip example

fd3eda2

datapythonista reviewed May 1, 2018

View reviewed changes

datapythonista mentioned this pull request May 1, 2018

DOC: improve the shared docstring of Series.str.strip() #20897

Closed

4 tasks

simplify lstrip and rstrip

c69b1e3

Cheukting force-pushed the Doc_str_strip branch from 260b482 to c69b1e3 Compare May 1, 2018 21:28

jreback added Docs Strings String extension data type and string data labels May 4, 2018

jreback requested changes May 4, 2018

View reviewed changes

Cheukting and others added 2 commits May 5, 2018 11:43

added np.nan example

5160feb

Merge remote-tracking branch 'upstream/master' into Doc_str_strip

0d6a4cc

mroeschke added this to the 0.24.0 milestone Jul 7, 2018

jreback approved these changes Jul 7, 2018

View reviewed changes

mroeschke merged commit 0add019 into pandas-dev:master Jul 7, 2018


		Striping a set of characters for Index

		>>> df = pd.DataFrame(index=['1.ant ','2._bee__','3. cat_'])


		>>> df = pd.DataFrame(index=['1.ant ','2._bee__','3. cat_'])

		>>> df.index

		Stripping whitespaces for Series

		>>> s = pd.Series([' ant', 'bee ', ' cat '])

Uh oh!

DOC: update the pandas.Series.str.strip docstring #20863

DOC: update the pandas.Series.str.strip docstring #20863

Uh oh!

Conversation

Cheukting commented Apr 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WillAyd left a comment

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!

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

Uh oh!

Comment last updated on July 07, 2018 at 20:05 Hours UTC

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Cheukting commented Apr 30, 2018

Uh oh!

datapythonista left a comment

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!

alsobay commented May 1, 2018

Uh oh!

Cheukting commented May 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mroeschke commented Jul 7, 2018

Uh oh!

jreback commented Jul 7, 2018

Uh oh!

mroeschke commented Jul 7, 2018

Uh oh!

Uh oh!

Cheukting commented Apr 29, 2018 •

edited

Loading

pep8speaks commented Apr 30, 2018 •

edited

Loading

codecov bot commented Apr 30, 2018 •

edited

Loading

Cheukting commented May 1, 2018 •

edited

Loading