-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
DOC: update the pandas.Series.dt.is_year_start docstring #20275
Conversation
pandas/core/indexes/datetimes.py
Outdated
@@ -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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
pandas/core/indexes/datetimes.py
Outdated
|
||
Returns | ||
------- | ||
is_year_start : Series of boolean. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
"
pandas/core/indexes/datetimes.py
Outdated
|
||
See Also | ||
-------- | ||
is_year_end : Return a boolean indicating whether the date is the |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
pandas/core/indexes/datetimes.py
Outdated
1 2017-12-31 | ||
2 2018-01-01 | ||
dtype: datetime64[ns] | ||
>>> dates.dt.is_year_start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line between cases
[ci skip]
And thanks again :) |
Checklist for the pandas documentation sprint (ignore this if you are doing
an unrelated PR):
scripts/validate_docstrings.py <your-function-or-method>
git diff upstream/master -u -- "*.py" | flake8 --diff
python doc/make.py --single <your-function-or-method>
Please include the output of the validation script below between the "```" ticks:
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.