Skip to content

DOC: Fix docstring of window.Expanding/Rolling.apply and document *args and **kwargs #24184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

LJArendse
Copy link
Contributor

@LJArendse LJArendse commented Dec 9, 2018

This pull request will

  • Fix docstring formatting for pandas.core.window.Expanding.apply and pandas.core.window.Rolling.apply

  • Document *args and **kwargs parameters:
    The following functions had *args and **kwargs added to their docstring:
    Rolling.max
    Expanding.max
    EWM.mean
    EWM.std
    EWM.var
    The following functions had **kwargs added to their docstring:
    Rolling.skew
    Expanding.skew
    Rolling.cov
    Expanding.cov
    EWM.cov
    EWM.corr

…and pandas.core.window.Rolling.apply

The following 'errors' were fixed:
- Summary does not start with a capital letter
- Unknown parameters {\*args and \*\*kwargs are passed to the function}
- Parameter "func" description should finish with "."
- Parameter "raw" description should start with a capital letter
- Parameter "\*args and \*\*kwargs are passed to the function" has no type
- Parameter "\*args and \*\*kwargs are passed to the function" description should finish with "."
- No Returns section found

The following 'warning' was addressed:
- See Also section not found
@pep8speaks
Copy link

pep8speaks commented Dec 9, 2018

Hello @LJArendse! Thanks for updating the PR.

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

Comment last updated on January 07, 2019 at 11:30 Hours UTC

@jreback jreback added the Docs label Dec 9, 2018
Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the comment, you have PEP8 issues in your code (tabs), it's probably worth using a validator in your editor.

Also, if instead of describing what you did, you can just copy the output of the validation script, that's more useful to review.

@datapythonista datapythonista changed the title DOC: Fix docstring formatting for pandas.core.window.Expanding.apply … DOC: Fix docstring of window.Expanding/Rolling.apply Dec 9, 2018
@codecov
Copy link

codecov bot commented Dec 9, 2018

Codecov Report

Merging #24184 into master will decrease coverage by 49.17%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #24184       +/-   ##
===========================================
- Coverage   92.21%   43.03%   -49.18%     
===========================================
  Files         162      162               
  Lines       51723    51723               
===========================================
- Hits        47695    22261    -25434     
- Misses       4028    29462    +25434
Flag Coverage Δ
#multiple ?
#single 43.03% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/window.py 28.93% <ø> (-67.47%) ⬇️
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/core/categorical.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/formats/html.py 0% <0%> (-98.62%) ⬇️
pandas/core/groupby/categorical.py 0% <0%> (-95.46%) ⬇️
pandas/io/sas/sas7bdat.py 0% <0%> (-91.17%) ⬇️
pandas/io/sas/sas_xport.py 0% <0%> (-90.15%) ⬇️
... and 120 more

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 3dbb137...764f8a8. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 9, 2018

Codecov Report

Merging #24184 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24184      +/-   ##
==========================================
- Coverage   92.38%   92.37%   -0.02%     
==========================================
  Files         166      166              
  Lines       52478    52382      -96     
==========================================
- Hits        48483    48389      -94     
+ Misses       3995     3993       -2
Flag Coverage Δ
#multiple 90.8% <100%> (-0.02%) ⬇️
#single 43.01% <100%> (-0.03%) ⬇️
Impacted Files Coverage Δ
pandas/core/window.py 96.4% <100%> (-0.01%) ⬇️
pandas/core/internals/construction.py 95.93% <0%> (-0.73%) ⬇️
pandas/core/nanops.py 94.36% <0%> (-0.55%) ⬇️
pandas/core/internals/concat.py 96.45% <0%> (-0.37%) ⬇️
pandas/core/arrays/datetimes.py 97.68% <0%> (-0.34%) ⬇️
pandas/core/indexes/datetimelike.py 98.52% <0%> (-0.32%) ⬇️
pandas/core/internals/blocks.py 94.3% <0%> (-0.2%) ⬇️
pandas/core/indexes/period.py 92.06% <0%> (-0.06%) ⬇️
pandas/core/arrays/categorical.py 95.43% <0%> (-0.05%) ⬇️
pandas/core/indexes/category.py 98.61% <0%> (-0.04%) ⬇️
... and 27 more

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 62506ca...8bedb9d. Read the comment docs.

@datapythonista
Copy link
Member

And if you can please provide the output of the validation script in the description, as mentioned before, that would be helpful.

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

For next time, it's useful if you also copy the rendered docstring in the description, not only the list of errors. As the docstring is splitted in different variables, and some values are not explicit (like %(name)s, it's some times easier to understand the rendered docstring.

@LJArendse
Copy link
Contributor Author

@datapythonista
will take a look at why the pandas-dev.pandas check is failing at line 2275. Could be the changes to _doc_template used in the decorator @appender(_doc_template) ...?

2018-12-09T21:02:08.3673583Z ##[error]pandas/core/window.py(2275,): error SS04: pandas.core.window.EWM.mean: Summary contains heading whitespaces
2018-12-09T21:02:09.3424503Z Validate docstrings (GL06, SS04, PR03, PR05, EX04) DONE
2018-12-09T21:02:09.3426271Z ##[error]Bash exited with code '1'.

@datapythonista
Copy link
Member

can you run ./scripts/validate_docstrings.py pandas.core.window.EWM.mean and see how the docstring looks like? that should give you an idea of the problem.

I think the problem it's indeed removing the indentation _doc_template.

…y'] docstring, and Remove @appender(_doc_template) decorator for rolling apply and expanding apply
@LJArendse
Copy link
Contributor Author

Fixed Summary contains heading whitespace for pandas.core.window.EWM.mean
Maintained fixes to pandas.core.window.Expanding.apply and pandas.core.window.Rolling.apply
(see below)

pandas.core.window.EWM.mean

python scripts/validate_docstrings.py pandas.core.window.EWM.mean

################################################################################
################### Docstring (pandas.core.window.EWM.mean)  ###################
################################################################################

Exponential weighted moving average.

Returns
-------
same type as input

See Also
--------
Series.ewm
DataFrame.ewm

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

3 Errors found:
        Parameters {*args, **kwargs} not documented
        Missing description for See Also "Series.ewm" reference
        Missing description for See Also "DataFrame.ewm" reference
2 Warnings found:
        No extended summary found
        No examples section found

pandas.core.window.Expanding.apply

################################################################################
################ Docstring (pandas.core.window.Expanding.apply) ################
################################################################################

The expanding function apply.

Parameters
----------
func : function
    Must produce a single value from an ndarray input if ``raw=True``
    or a Series if ``raw=False``.
raw : bool, default None
    * ``False`` : passes each row or column as a Series to the
      function.
    * ``True`` or ``None`` : the passed function will receive ndarray
      objects instead.
      If you are just applying a NumPy reduction function this will
      achieve much better performance.

    The `raw` parameter is required and will show a FutureWarning if
    not passed. In the future `raw` will default to False.

    .. versionadded:: 0.23.0
*args, **kwargs :
    Arguments and keyword arguments to be passed into func.

Returns
-------
Same type as input

See Also
--------
Series.expanding : Series expanding.
DataFrame.expanding : DataFrame expanding.

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

3 Errors found:
        Parameters {kwargs, args} not documented
        Unknown parameters {*args, **kwargs :}
        Parameter "raw" description should start with a capital letter
2 Warnings found:
        No extended summary found
        No examples section found

pandas.core.window.Rolling.apply

################################################################################
################# Docstring (pandas.core.window.Rolling.apply) #################
################################################################################

The rolling function apply.

Parameters
----------
func : function
    Must produce a single value from an ndarray input if ``raw=True``
    or a Series if ``raw=False``.
raw : bool, default None
    * ``False`` : passes each row or column as a Series to the
      function.
    * ``True`` or ``None`` : the passed function will receive ndarray
      objects instead.
      If you are just applying a NumPy reduction function this will
      achieve much better performance.

    The `raw` parameter is required and will show a FutureWarning if
    not passed. In the future `raw` will default to False.

    .. versionadded:: 0.23.0
*args, **kwargs :
    Arguments and keyword arguments to be passed into func.

Returns
-------
Same type as input

See Also
--------
Series.rolling : Series rolling.
DataFrame.rolling : DataFrame rolling.

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

3 Errors found:
        Parameters {args, kwargs} not documented
        Unknown parameters {*args, **kwargs :}
        Parameter "raw" description should start with a capital letter
2 Warnings found:
        No extended summary found
        No examples section found

@LJArendse
Copy link
Contributor Author

@datapythonista looks like the fixes have passed the checks :)

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @LJArendse

@datapythonista datapythonista self-assigned this Dec 11, 2018
@WillAyd
Copy link
Member

WillAyd commented Dec 11, 2018 via email

@LJArendse
Copy link
Contributor Author

@WillAyd I agree, updated the _doc_template as discussed.

@datapythonista
Copy link
Member

what is the status of this? anything pending? @LJArendse @WillAyd

@LJArendse
Copy link
Contributor Author

Hi @datapythonista
The work for fixing the docstring of window.Expanding/Rolling.apply is done.

Additional to this, from what I understand, I will be investigating the best way to add *args, **kwargs to _doc_template. For the moment it is looking like I will add *args, **kwargs to _doc_template and then modify the docstrings of skew, cov, and corr. If you still want me to do this investigation, should the title of the pull request be modified to reflect these additional changes being made?

I will only be able to work on this after the 2nd of Jan.

@LJArendse
Copy link
Contributor Author

In the progress of adding the *args and **kwargs parameters to the following:

rolling.max - *args and **kwargs
expanding.max - *args and **kwargs

rolling.cov - **kwargs
expanding.cov - **kwargs
EWM.cov - **kwargs

EWM.mean - *args and **kwargs
EWM.std - *args and **kwargs
EWM.var - *args and **kwargs
EWM.corr - **kwargs

@datapythonista
Copy link
Member

lgtm, let us know when you're done.

And feel free to leave some of the docstring fixes to other PRs, in general it's better to have smaller PRs, than a large one.

@WillAyd if you can take a look.

@LJArendse
Copy link
Contributor Author

Done. I am busy uploading the before and after validation output for each updated docstring.

@LJArendse
Copy link
Contributor Author

LJArendse commented Jan 7, 2019

Expanding.apply

Before:
python scripts/validate_docstrings.py pandas.core.window.Expanding.apply

################################################################################
################ Docstring (pandas.core.window.Expanding.apply) ################
################################################################################

expanding function apply.

Parameters
----------
func : function
    Must produce a single value from an ndarray input if ``raw=True``
    or a Series if ``raw=False``
raw : bool, default None
    * ``False`` : passes each row or column as a Series to the
      function.
    * ``True`` or ``None`` : the passed function will receive ndarray
      objects instead.
      If you are just applying a NumPy reduction function this will
      achieve much better performance.

    The `raw` parameter is required and will show a FutureWarning if
    not passed. In the future `raw` will default to False.

    .. versionadded:: 0.23.0

\*args and \*\*kwargs are passed to the function
        Returns
        -------
        same type as input

        See Also
        --------
        Series.expanding
        DataFrame.expanding

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

8 Errors found:
        Summary does not start with a capital letter
        Parameters {kwargs, args} not documented
        Unknown parameters {\*args and \*\*kwargs are passed to the function}
        Parameter "func" description should finish with "."
        Parameter "raw" description should start with a capital letter
        Parameter "\*args and \*\*kwargs are passed to the function" has no type
        Parameter "\*args and \*\*kwargs are passed to the function" description should finish with "."
        No Returns section found
3 Warnings found:
        No extended summary found
        See Also section not found
        No examples section found

After:

################################################################################
################ Docstring (pandas.core.window.Expanding.apply) ################
################################################################################

The expanding function's apply function.

Parameters
----------
func : function
    Must produce a single value from an ndarray input if ``raw=True``
    or a Series if ``raw=False``.
raw : bool, default None
    * ``False`` : passes each row or column as a Series to the
      function.
    * ``True`` or ``None`` : the passed function will receive ndarray
      objects instead.
      If you are just applying a NumPy reduction function this will
      achieve much better performance.

    The `raw` parameter is required and will show a FutureWarning if
    not passed. In the future `raw` will default to False.

    .. versionadded:: 0.23.0
*args, **kwargs
    Arguments and keyword arguments to be passed into func.

Returns
-------
Series or DataFrame
    Return type is determined by the caller.

See Also
--------
Series.expanding : Series expanding.
DataFrame.expanding : DataFrame expanding.

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

3 Errors found:
        Parameters {kwargs, args} not documented
        Unknown parameters {*args, **kwargs}
        Parameter "raw" description should start with a capital letter
2 Warnings found:
        No extended summary found
        No examples section found

Final output:
expanding_apply

@LJArendse
Copy link
Contributor Author

LJArendse commented Jan 7, 2019

Rolling.apply

Before:
python scripts/validate_docstrings.py pandas.core.window.Rolling.apply

################################################################################
################# Docstring (pandas.core.window.Rolling.apply) #################
################################################################################

rolling function apply.

Parameters
----------
func : function
    Must produce a single value from an ndarray input if ``raw=True``
    or a Series if ``raw=False``
raw : bool, default None
    * ``False`` : passes each row or column as a Series to the
      function.
    * ``True`` or ``None`` : the passed function will receive ndarray
      objects instead.
      If you are just applying a NumPy reduction function this will
      achieve much better performance.

    The `raw` parameter is required and will show a FutureWarning if
    not passed. In the future `raw` will default to False.

    .. versionadded:: 0.23.0

\*args and \*\*kwargs are passed to the function
        Returns
        -------
        same type as input

        See Also
        --------
        Series.rolling
        DataFrame.rolling

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

8 Errors found:
        Summary does not start with a capital letter
        Parameters {args, kwargs} not documented
        Unknown parameters {\*args and \*\*kwargs are passed to the function}
        Parameter "func" description should finish with "."
        Parameter "raw" description should start with a capital letter
        Parameter "\*args and \*\*kwargs are passed to the function" has no type
        Parameter "\*args and \*\*kwargs are passed to the function" description should finish with "."
        No Returns section found
3 Warnings found:
        No extended summary found
        See Also section not found
        No examples section found

After:

################################################################################
################# Docstring (pandas.core.window.Rolling.apply) #################
################################################################################

The rolling function's apply function.

Parameters
----------
func : function
    Must produce a single value from an ndarray input if ``raw=True``
    or a Series if ``raw=False``.
raw : bool, default None
    * ``False`` : passes each row or column as a Series to the
      function.
    * ``True`` or ``None`` : the passed function will receive ndarray
      objects instead.
      If you are just applying a NumPy reduction function this will
      achieve much better performance.

    The `raw` parameter is required and will show a FutureWarning if
    not passed. In the future `raw` will default to False.

    .. versionadded:: 0.23.0
*args, **kwargs
    Arguments and keyword arguments to be passed into func.

Returns
-------
Series or DataFrame
    Return type is determined by the caller.

See Also
--------
Series.rolling : Series rolling.
DataFrame.rolling : DataFrame rolling.

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

3 Errors found:
        Parameters {args, kwargs} not documented
        Unknown parameters {*args, **kwargs}
        Parameter "raw" description should start with a capital letter
2 Warnings found:
        No extended summary found
        No examples section found

Final output:
rolling_apply

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks @LJArendse

@WillAyd merge when you're happy

@LJArendse LJArendse changed the title DOC: Fix docstring of window.Expanding/Rolling.apply DOC: Fix docstring of window.Expanding/Rolling.apply and document *args and **kwargs Jan 7, 2019
@LJArendse
Copy link
Contributor Author

The following functions had *args and **kwargs added to their docstring:
Rolling.max
Expanding.max
EWM.mean
EWM.std
EWM.var

I can upload the full before and after validation output if you want (I did not want to upload for each as it will make this thread extra long - unless you are okay with that?).

The final output for each looks as follows:
rolling_max
expanding_max
ewm_mean
ewm_std
ewm_var

@LJArendse
Copy link
Contributor Author

The following functions had **kwargs added to their docstring:
Rolling.skew
Expanding.skew
Rolling.cov
Expanding.cov
EWM.cov
EWM.corr

I can upload the full before and after validation output if you want (I did not want to upload for each as it will make the thread extra long - unless you are okay with that?).

The final output for each looks as follows:
rolling_skew
expanding_skew
rolling_cov
expanding_cov
ewm_cov
ewm_corr

@WillAyd WillAyd merged commit 4bf3a0e into pandas-dev:master Jan 7, 2019
@WillAyd
Copy link
Member

WillAyd commented Jan 7, 2019

Thanks @LJArendse

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants