Skip to content

DOC: update the pandas.Series.dt.is_year_start docstring #20275

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

Merged
merged 3 commits into from
Mar 14, 2018

Conversation

janelle
Copy link
Contributor

@janelle janelle commented Mar 11, 2018

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.dt.is_year_start) ##################
################################################################################

Return a boolean indicating whether the date is the first day of the
year.

Returns
-------
is_year_start : Series of boolean.

See Also
--------
is_year_end : Return a boolean indicating whether the date is the
    last day of the year.

Examples
--------
>>> dates = pd.Series(pd.date_range("2017-12-30", periods=3))
>>> dates
0   2017-12-30
1   2017-12-31
2   2018-01-01
dtype: datetime64[ns]
>>> dates.dt.is_year_start
0    False
1    False
2    True
dtype: bool

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

Errors found:
    No summary found (a short summary in a single line should be present at the beginning of the docstring)

If the validation script still gives errors, but you think there is a good reason
to deviate in this case (and there are certainly such cases), please state this
explicitly.

  • short summary exceeds one line by one word, hope that is ok

@@ -1743,7 +1743,33 @@ def freq(self, value):
is_year_start = _field_accessor(
'is_year_start',
'is_year_start',
"Logical indicating if first day of year (defined by frequency)")
"""
Return a boolean indicating whether the date is the first day of the
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing about the single line.

Copy link
Member

Choose a reason for hiding this comment

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

General question here: for attributes, do we want to follow our rule of starting the summary line with a verb?

I have the feeling that if all those attributes start with "Return", that's rather superfluous

Copy link
Member

Choose a reason for hiding this comment

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

For example, this gives now a nice overview table with short explanations: http://pandas.pydata.org/pandas-docs/stable/api.html#time-date-components

Personally, I would say that if all those "The year of the datetime" become "Return the year of the datetime" etc, this is not necessarily an improvement

cc @datapythonista your view here is very appreciated

Copy link
Member

Choose a reason for hiding this comment

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

I agree, I think that policy only makes sense to functions and methods, not to classes, properties or attributes.

If I'm not wrong, that is said in the documentation, and also the validation avoids the check for the infinitive verb.


Returns
-------
is_year_start : Series of boolean.
Copy link
Contributor

Choose a reason for hiding this comment

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

is_year_end : Series or DatetimeIndex
    The same type as the original data. Series will have the same
    name and index. DatetimeIndex will have the same name.

0 False
1 False
2 True
dtype: bool
Copy link
Contributor

Choose a reason for hiding this comment

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

Second example showing DatetimeIndex.is_year_start.

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 then also write something like "This attribute is both available on a DatetimeIndex as a Series with datetime values" or for the Series example: "On a Series with datetime values, the attribute can be accessed with .dt"


See Also
--------
is_year_end : Return a boolean indicating whether the date is the
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this will resolve correctly, @jorisvandenbossche do you know?

I think it should be Series.dt.is_year_start

or DatetimeIndex.is_year_start. Don't think we've decided what to do here...

Copy link
Member

Choose a reason for hiding this comment

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

Well, I was just testing it :-)
Turns out this works (because it is the same class, it resolves OK), but I wanted to comment that for clarity I would write it in full like DatetimeIndex.is_year_start, but on the other hand, not writing it full ensures it links correctly in both Series.dt and DatetimeIndex case

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, in that case I'm ok with leaving it "ambiguous" since in the HTML docs you can click through and get the correct one. In the REPL you have tab completion.

1 2017-12-31
2 2018-01-01
dtype: datetime64[ns]
>>> dates.dt.is_year_start
Copy link
Contributor

Choose a reason for hiding this comment

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

blank line between cases

@TomAugspurger TomAugspurger merged commit 5c6c5f6 into pandas-dev:master Mar 14, 2018
@TomAugspurger
Copy link
Contributor

And thanks again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants