DOC: update the pandas.Series.str.slice_replace docstring #20273

techedlaksh · 2018-03-11T07:50:32Z

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:

################################################################################
################# Docstring (pandas.Series.str.slice_replace)  #################
################################################################################

Replace a sliced string.

Replace a slice of each string in the Series/Index with another
string.

Parameters
----------
start : int or None
    Left edge index.
stop : int or None
    Right edge index.
repl : str or None
    String for replacement.

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

Examples
--------
>>> s = pd.Series(['This is a Test 1', 'This is a Test 2'])
>>> s
0    This is a Test 1
1    This is a Test 2
dtype: object
>>> s = s.str.slice_replace(8, 14, 'an Example')
>>> s
0    This is an Example 1
1    This is an Example 2
dtype: object

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

Errors found:
	See Also section not found

TomAugspurger

Minor suggestions, if you have a chance to update it'd be great. Looks good though!

TomAugspurger · 2018-03-11T11:24:07Z

pandas/core/strings.py

@@ -1180,19 +1180,36 @@ def str_slice(arr, start=None, stop=None, step=None):

 def str_slice_replace(arr, start=None, stop=None, repl=None):
    """
+    Replace a sliced string.


Maybe it's implied, but I'd like to see this say what the sliced bit is replaced with

Replace a sliced string with another value. Like all of pandas string methods, this operates elementwise on each string in the Series or Index.

and then the rest of the docstring.

I think "a slice of a string" is easier to understand than "sliced string"

TomAugspurger · 2018-03-11T11:26:24Z

pandas/core/strings.py


    Returns
    -------
    replaced : Series/Index of objects
+


For a See Also, str.replace to link to https://docs.python.org/3/library/stdtypes.html#str.replace would be good.

And we also have replace as a string method in Series.str, so I would rather add a link to that?

jorisvandenbossche

Thanks! Added some additional comments

jorisvandenbossche · 2018-03-11T12:31:01Z

pandas/core/strings.py

@@ -1180,19 +1180,36 @@ def str_slice(arr, start=None, stop=None, step=None):

 def str_slice_replace(arr, start=None, stop=None, repl=None):
    """
+    Replace a sliced string.


I think "a slice of a string" is easier to understand than "sliced string"

jorisvandenbossche · 2018-03-11T12:32:06Z

pandas/core/strings.py

    Replace a slice of each string in the Series/Index with another
    string.

    Parameters
    ----------
    start : int or None
+        Left edge index.


Can you indicate what None means? (I suppose this is the from the start of the string)

Can you also change "int or None" into "int, optional"? And then maybe you can say for the above "If not specified, slice from the start of each string"

(and same for end, but then "slice until the end of each string"

jorisvandenbossche · 2018-03-11T12:33:37Z

pandas/core/strings.py


    Returns
    -------
    replaced : Series/Index of objects
+


And we also have replace as a string method in Series.str, so I would rather add a link to that?

jreback · 2018-03-11T13:18:57Z

pandas/core/strings.py

+    0    This is a Test 1
+    1    This is a Test 2
+    dtype: object
+    >>> s = s.str.slice_replace(8, 14, 'an Example')


blank line between cases

TomAugspurger · 2018-03-14T19:16:52Z

Updated if you want to take a look.

I'm not sure how, but apparently changing the default repl to '' broke tests. 7a0cf0c reverted that to be how it was before. Will let the tests run on this before merging.

jorisvandenbossche · 2018-03-14T22:29:03Z

pandas/core/strings.py

-    repl : str or None
-        String for replacement.
+    start : int, optional
+        Left index position to use for the slice. The default of None


General question: do we more want to use the phrase of "If not specified" instead of "The default of None" if we say something is optional?
On the one hand I think this can be clearer as that is what you do in practice, you don't specify it. But on the other hand of course the signature shows None.

Yeah, if not specified is probably best.

Went with if not specified (None)

Ah, If not specified (None), is a good combination of both, we should use that pattern more

[ci skip]

…ring_slice_replace [ci skip] [ci skip]

codecov · 2018-03-16T11:47:17Z

Codecov Report

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

@@            Coverage Diff            @@
##             master   #20273   +/-   ##
=========================================
  Coverage          ?   91.77%           
=========================================
  Files             ?      152           
  Lines             ?    49178           
  Branches          ?        0           
=========================================
  Hits              ?    45133           
  Misses            ?     4045           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.16% <ø> (?)`
#single	`41.84% <ø> (?)`

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

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 ea8ac51...5263522. Read the comment docs.

update doc for method slice_replace

916f4cb

jorisvandenbossche added Docs and removed Docs labels Mar 11, 2018

TomAugspurger approved these changes Mar 11, 2018

View reviewed changes

jorisvandenbossche reviewed Mar 11, 2018

View reviewed changes

jreback added the Strings String extension data type and string data label Mar 11, 2018

jreback requested changes Mar 11, 2018

View reviewed changes

TomAugspurger added 2 commits March 14, 2018 14:12

Updated

bbed286

Fix tests

7a0cf0c

jorisvandenbossche reviewed Mar 14, 2018

View reviewed changes

TomAugspurger added 2 commits March 16, 2018 06:46

Updated [ci skip]

56a854e

[ci skip]

Merge remote-tracking branch 'upstream/master' into techedlaksh-docst…

5263522

…ring_slice_replace [ci skip] [ci skip]

TomAugspurger added this to the 0.23.0 milestone Mar 16, 2018

TomAugspurger merged commit 03879ba into pandas-dev:master Mar 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: update the pandas.Series.str.slice_replace docstring #20273

DOC: update the pandas.Series.str.slice_replace docstring #20273

techedlaksh commented Mar 11, 2018 •

edited

Loading

TomAugspurger left a comment

TomAugspurger Mar 11, 2018

jorisvandenbossche Mar 11, 2018

TomAugspurger Mar 11, 2018

jorisvandenbossche Mar 11, 2018

jorisvandenbossche left a comment

jorisvandenbossche Mar 11, 2018

jorisvandenbossche Mar 11, 2018

jorisvandenbossche Mar 11, 2018

jreback Mar 11, 2018

TomAugspurger commented Mar 14, 2018

jorisvandenbossche Mar 14, 2018

TomAugspurger Mar 16, 2018

TomAugspurger Mar 16, 2018

jorisvandenbossche Mar 17, 2018

codecov bot commented Mar 16, 2018

DOC: update the pandas.Series.str.slice_replace docstring #20273

DOC: update the pandas.Series.str.slice_replace docstring #20273

Conversation

techedlaksh commented Mar 11, 2018 • edited Loading

TomAugspurger 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

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

TomAugspurger commented Mar 14, 2018

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 Mar 16, 2018

Codecov Report

techedlaksh commented Mar 11, 2018 •

edited

Loading