Skip to content

DOC" update the Pandas core window rolling count docstring" #20264

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Mar 19, 2018
Merged

DOC" update the Pandas core window rolling count docstring" #20264

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6f59df9
updated windows.py count docstring
tommy-stone Mar 10, 2018
4c1500c
DOC: update the pandas.core.window.Rolling.count docstring
tommy-stone Mar 11, 2018
a59c9cb
Received comments on updating description to use count, return type a…
tommy-stone Mar 17, 2018
122f930
updated See Also section to remove typo with calling objects
tommy-stone Mar 17, 2018
f8d77ef
updated count docstring based on comments
tommy-stone Mar 18, 2018
946836c
also remove doc_template for expanding
jorisvandenbossche Mar 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions pandas/core/window.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,8 +780,41 @@ def calc(x):

class _Rolling_and_Expanding(_Rolling):

_shared_docs['count'] = """%(name)s count of number of non-NaN
observations inside provided window."""
_shared_docs['count'] = dedent(r"""
The %(name)s sum if it is a non-Nan value inside the window.
Copy link
Member

Choose a reason for hiding this comment

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

Using the word sum is confusing here for a count. Can you reword to the effect of "count of non-NaN observations..."?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"count of non-Nan observations inside the rolling window" would work?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

@scriptomation be sure that you actually surround your reference to NaN in back ticks, so literally you would write non-`NaN` (capitalization matters too)


Returns
-------
Returns the object type with the sum of the window values
Copy link
Member

Choose a reason for hiding this comment

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

Can you make note of what the return type is? Also similar comment as above - referring to sum within a count function is confusing. I'm not sure current row is the right concept here, though I understand what you are trying to get at

Copy link
Member

Choose a reason for hiding this comment

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

You can use a similar wording as here: https://github.com/pandas-dev/pandas/pull/20263/files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. How does this work?

Returns
-------
Series or Dataframe
Returned object type is determined by the caller of %(name)

Copy link
Member

Choose a reason for hiding this comment

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

I would just copy verbatim, so Returned object type is determined by the caller of the %(name)s calculation

in the current row.

See Also
--------
Series.%(name)s : Calling object with Series data
DataFrame.%(name)s : Calling object with DataFrames
Copy link
Member

Choose a reason for hiding this comment

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

Can you link to Series / DataFrame sum methods?

Copy link
Member

Choose a reason for hiding this comment

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

You can take a look at how the other PRs did this, see eg https://github.com/pandas-dev/pandas/pull/19999/files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I will update that information.


Examples
--------
>>> s = pd.Series([2, 3, np.nan, 'values'])
Copy link
Member

Choose a reason for hiding this comment

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

It's a bit strange to give an example with mixed floats / strings (I mean, in general that's not really good practice to have data like that). I think only using floats (with nans) is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to show an example of mixed values being counted, in case a mistake is made by having different types of data. I see what you are saying though since it's already stated in the description. I will update to only use floats and nans. Thanks!

>>> s.rolling(2).count()
0 1.0
1 2.0
2 1.0
3 1.0
dtype: float64
>>> s.rolling(3).count()
0 1.0
1 2.0
2 2.0
3 2.0
dtype: float64
>>> s.rolling(4).count()
0 1.0
1 2.0
2 2.0
3 3.0
dtype: float64
""")

def count(self):

Expand Down Expand Up @@ -1199,7 +1232,6 @@ def aggregate(self, arg, *args, **kwargs):
agg = aggregate

@Substitution(name='rolling')
@Appender(_doc_template)
@Appender(_shared_docs['count'])
def count(self):

Expand Down