DOC: Update series apply docstring. GH22459 #22510

eldritchideen · 2018-08-26T03:07:06Z

refs DOC: ongoing fixing of doctests #22459
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Updated Series.apply docstring to resolve errors raised from scripts/validate_docstrings.py from #22459.

codecov · 2018-08-26T03:58:05Z

Codecov Report

Merging #22510 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #22510   +/-   ##
=======================================
  Coverage   92.29%   92.29%           
=======================================
  Files         161      161           
  Lines       51498    51498           
=======================================
  Hits        47530    47530           
  Misses       3968     3968

Flag	Coverage Δ
#multiple	`90.69% <ø> (ø)`	⬆️
#single	`42.43% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/series.py	`93.68% <ø> (ø)`	⬆️

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 f2839fe...d5c13b2. Read the comment docs.

gfyoung · 2018-08-27T09:31:04Z

pandas/core/series.py

        args : tuple
-            Positional arguments to pass to function in addition to the value
-        Additional keyword arguments will be passed as keywords to the function
+            Positional arguments to pass to func in addition to the value.


"in addition to the value" - can we clarify what that means?

gfyoung · 2018-08-27T09:32:12Z

pandas/core/series.py

-            Positional arguments to pass to function in addition to the value
-        Additional keyword arguments will be passed as keywords to the function
+            Positional arguments to pass to func in addition to the value.
+        **kwds : dict


@datapythonista : I wonder if we should standardize and use **kwargs everywhere.

(@eldritchideen : Don't worry about this comment. This is a larger question beyond this PR for the time being.)

In terms of documentation, the standard we defined was to always document *args and **kwargs if present, showing the stars in the name, but without the type (it'll always be tuple and dict).

Not sure if you mean using kwargs instead of kwds, or which standardization are you proposing here, but I'm +1 on being consistent everywhere.

I see that in this case args needs to be provided as a tuple (not as positional arguments) and kwds needs to be provided as keyword arguments (not as a dict). Not sure if there is a reason for this, but if it's not, I'd be in favor of changing it to be consistent (in a separate PR). @jreback ?

@datapythonista : Indeed, I was referring to using kwargs across the board. I'm also in favor of consistency in general.

That sounds good to me, and afaik it should be a backward compatible change in all cases, so +1 on this. Do you want to create an issue?

datapythonista

Changes look good to me, after considering @gfyoung comments, and removing the type (dict) of **kwds. Thanks @eldritchideen

datapythonista · 2018-08-27T12:40:39Z

pandas/core/series.py

-            Positional arguments to pass to function in addition to the value
-        Additional keyword arguments will be passed as keywords to the function
+            Positional arguments to pass to func in addition to the value.
+        **kwds : dict


In terms of documentation, the standard we defined was to always document *args and **kwargs if present, showing the stars in the name, but without the type (it'll always be tuple and dict).

Not sure if you mean using kwargs instead of kwds, or which standardization are you proposing here, but I'm +1 on being consistent everywhere.

I see that in this case args needs to be provided as a tuple (not as positional arguments) and kwds needs to be provided as keyword arguments (not as a dict). Not sure if there is a reason for this, but if it's not, I'd be in favor of changing it to be consistent (in a separate PR). @jreback ?

eldritchideen · 2018-08-28T10:16:27Z

@datapythonista I removed the type from the **kwds parameter. I also agree that it should be renamed to **kwargs to be more consistent with the rest of the code base.

datapythonista

Thanks for the changes @eldritchideen . Can you add the clarification to the args parameter @gfyoung asked? I don't think it's clear enough either.

Also, it'd be great if you can change couple of few things to make the docstring follow all our guidelines:

Remove the colon after **kwds
In the Returns section, remove the y : and leave just Series or DataFrame adding the description (and clarifications about the type) to the next line (indented)
Add a period at the end of each See Also item (and capitalize the A in Also in the title)
Remove the blank line after the Examples title.
Rename the variable series by s in the examples.

Thank you!

pep8speaks · 2018-09-04T10:54:35Z

Hello @eldritchideen! Thanks for updating the PR.

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

Comment last updated on September 04, 2018 at 10:56 Hours UTC

eldritchideen · 2018-09-04T10:57:16Z

I have updated the Series.apply docstring based on review feedback. Thanks @datapythonista and @gfyoung

datapythonista

Looking better, there are some more changes that we can do (mainly to the original code) to make it look perfect.

datapythonista · 2018-09-04T12:18:52Z

pandas/core/series.py


        Parameters
        ----------
        func : function
-        convert_dtype : boolean, default True
+            Python function or NumPy ufunc.


may be we can say "Python function or NumPy ufunc to apply."?

datapythonista · 2018-09-04T12:20:03Z

pandas/core/series.py

-        Series.transform: only perform transforming type operations
+        Series.map: For element-wise operations.
+        Series.agg: only perform aggregating type operations.
+        Series.transform: only perform transforming type operations.


Can you capitalize all the first letters of the descriptions

datapythonista · 2018-09-04T12:22:58Z

pandas/core/series.py

@@ -3144,7 +3146,7 @@ def apply(self, func, convert_dtype=True, args=(), **kwds):

        >>> def square(x):
        ...     return x**2


Can you fix the PEP-8 of all examples please? There are missing spaces around all the ** , one - and one +=. I know it's from the original code, but let's get it fixed.

datapythonista · 2018-09-04T12:23:49Z

pandas/core/series.py

        Create a series with typical summer temperatures for each city.

-        >>> series = pd.Series([20, 21, 12], index=['London',
+        >>> s = pd.Series([20, 21, 12], index=['London',
        ... 'New York','Helsinki'])


I think it's more readable if we have all index in the next line.

datapythonista · 2018-09-04T12:26:16Z

pandas/core/series.py

@@ -3104,36 +3104,38 @@ def apply(self, func, convert_dtype=True, args=(), **kwds):
        """
        Invoke function on values of Series. Can be ufunc (a NumPy function
        that applies to the entire Series) or a Python function that only works
-        on single values
+        on single values.


Can you have a single line description first, and the rest later. You have the documentation about this here:
https://pandas.pydata.org/pandas-docs/stable/contributing_docstring.html

If you run ./scripts/validate_docstrings.py pandas.Series.apply should report an error about it.

jorisvandenbossche · 2018-09-25T16:51:28Z

@eldritchideen do you have time to update this PR based on the comments?

WillAyd · 2018-11-23T03:38:32Z

@datapythonista I see the self assignment on this - are you actually working this one?

datapythonista · 2018-11-24T15:11:44Z

I forgot about it, but fixed the pending issues now. Can you take a look and merge if everything is ok (or fix any outstanding issue, as the PR is discontinued)?

WillAyd

Thanks @datapythonista will merge on green

datapythonista · 2018-11-24T20:00:58Z

thanks @eldritchideen

eldritchideen added 2 commits August 26, 2018 13:01

DOC: Updated Series.apply docstring. GH22459

9b17810

DOC: Updated wording.

817379f

gfyoung reviewed Aug 27, 2018

View reviewed changes

gfyoung requested a review from datapythonista August 27, 2018 09:32

gfyoung added Docs Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff labels Aug 27, 2018

datapythonista reviewed Aug 27, 2018

View reviewed changes

DOC: Removed type from **kwds parameter.

5baa4fd

datapythonista reviewed Aug 28, 2018

View reviewed changes

DOC: Updated Series.apply docstring based on review feedback.

d07650f

DOC: Removed trailing whitespace.

0eb4e8c

datapythonista reviewed Sep 4, 2018

View reviewed changes

datapythonista self-assigned this Nov 3, 2018

Merging from master, and last comments from review addressed

d5c13b2

WillAyd approved these changes Nov 24, 2018

View reviewed changes

datapythonista merged commit d865e52 into pandas-dev:master Nov 24, 2018

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

DOC: Update Series.apply docstring (pandas-dev#22510)

b7eaacb

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

DOC: Update Series.apply docstring (pandas-dev#22510)

9b004ef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Update series apply docstring. GH22459 #22510

DOC: Update series apply docstring. GH22459 #22510

eldritchideen commented Aug 26, 2018 •

edited by datapythonista

Loading

codecov bot commented Aug 26, 2018 •

edited

Loading

gfyoung Aug 27, 2018

gfyoung Aug 27, 2018 •

edited

Loading

datapythonista Aug 27, 2018

gfyoung Aug 27, 2018

datapythonista Aug 27, 2018

datapythonista left a comment

datapythonista Aug 27, 2018

eldritchideen commented Aug 28, 2018

datapythonista left a comment

pep8speaks commented Sep 4, 2018 •

edited

Loading

eldritchideen commented Sep 4, 2018

datapythonista left a comment

datapythonista Sep 4, 2018

datapythonista Sep 4, 2018

datapythonista Sep 4, 2018

datapythonista Sep 4, 2018

datapythonista Sep 4, 2018

jorisvandenbossche commented Sep 25, 2018

WillAyd commented Nov 23, 2018

datapythonista commented Nov 24, 2018

WillAyd left a comment

datapythonista commented Nov 24, 2018

		@@ -3144,7 +3146,7 @@ def apply(self, func, convert_dtype=True, args=(), **kwds):

		>>> def square(x):
		... return x**2

DOC: Update series apply docstring. GH22459 #22510

DOC: Update series apply docstring. GH22459 #22510

Conversation

eldritchideen commented Aug 26, 2018 • edited by datapythonista Loading

codecov bot commented Aug 26, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

gfyoung Aug 27, 2018 • 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

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eldritchideen commented Aug 28, 2018

datapythonista left a comment

Choose a reason for hiding this comment

pep8speaks commented Sep 4, 2018 • edited Loading

Comment last updated on September 04, 2018 at 10:56 Hours UTC

eldritchideen commented Sep 4, 2018

datapythonista 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

jorisvandenbossche commented Sep 25, 2018

WillAyd commented Nov 23, 2018

datapythonista commented Nov 24, 2018

WillAyd left a comment

Choose a reason for hiding this comment

datapythonista commented Nov 24, 2018

eldritchideen commented Aug 26, 2018 •

edited by datapythonista

Loading

codecov bot commented Aug 26, 2018 •

edited

Loading

gfyoung Aug 27, 2018 •

edited

Loading

pep8speaks commented Sep 4, 2018 •

edited

Loading