DOC: update the Period.dayofyear docstring #20277
Conversation
|
@jreback please review 🙂 |
pandas/_libs/tslibs/period.pyx
Outdated
| Returns | ||
| ------- | ||
| int | ||
| The day of year. |
There was a problem hiding this comment.
This should be indented 4 space.
pandas/_libs/tslibs/period.pyx
Outdated
| -------- | ||
| Period.dayofweek : Return the day of week. | ||
| Period.daysinmonth : Return the days in that month. | ||
|
|
There was a problem hiding this comment.
Add PeriodIndex.dayofyear
There was a problem hiding this comment.
Do you mean 'weekofyear' ? (dayofyear is the current docstring)
| @@ -1255,6 +1255,33 @@ cdef class _Period(object): | |||
|
|
|||
| @property | |||
| def dayofyear(self): | |||
| """ | |||
| Return the day of the year. | |||
There was a problem hiding this comment.
Periods represent time spans so it's good to note that this is the start of the period. So maybe
Return the day of the year the of the Period's start.
That's a bit wordy though. Feel free to reword if you have a better phrasing.
There was a problem hiding this comment.
sorry, but I didn't understand the meaning of the sentence Return the day of the year the of the Period's start., specifically of the period's start
There was a problem hiding this comment.
Period is a span, so saying "Day of the year" is a bit ambiguous. Is it the start, end, somewhere in between?
If you have a period like `Period('2017-01-01', freq='A')
In [2]: pd.Period('2017-01-01', freq='A')
Out[2]: Period('2017', 'A-DEC')
In [3]: pd.Period('2017-01-01', freq='A').day
Out[3]: 31I was apparently wrong about it always being the start. I guess it's related to freq
There was a problem hiding this comment.
This comment will be relevant to many of the Period attributes like day, dayofyear, .. (also the ones I already merged ..).
So it would be good to be consistent in it.
The annual frequency is by default a "year-end" frequency, so it's indeed freq related.
There was a problem hiding this comment.
I'm sorry, I don't follow... Can you show two cases creating a period starting on the same day but with different frequencies, and how this affect .day, .dayofweek, .dayofyear, etc.?
There was a problem hiding this comment.
@dukebody small example:
In [30]: p1 = pd.Period('2017-01-01', freq='D')
In [31]: p2 = pd.Period('2017-01-01', freq='M')
In [32]: p1
Out[32]: Period('2017-01-01', 'D')
In [33]: p2
Out[33]: Period('2017-01', 'M')
In [34]: p1.start_time
Out[34]: Timestamp('2017-01-01 00:00:00')
In [35]: p2.start_time
Out[35]: Timestamp('2017-01-01 00:00:00')
In [36]: p1.dayofyear
Out[36]: 1
In [37]: p2.dayofyear
Out[37]: 31
I am not really familiar with this frequency business, but you have certain frequences which are "anchored" at the beginning of the period, and others at the end of the period. But I don't directly see a way how to inspect this.
There was a problem hiding this comment.
That's quite confusing. Should we open a ticket to make .dayofyear etc. always refer to the start time of the period? Not to the "anchored" time. Otherwise these functions are very unpredictable IMO and therefore don't add a lot of value.
There was a problem hiding this comment.
Yes, I am writing one currently ;)
I suppose those attributes will mainly be useful for frequencies <= 1D
There was a problem hiding this comment.
I'm OK with merging this PR without referring to the start or end date of the period, and then work on code to make this predictable and change all docstrings to refer to the period start date.
Signed-off-by: Tushar Mittal <[email protected]>
|
made the required changes, please review @jorisvandenbossche 🙂 |
Let's merge this, and will open a new issue to further discuss the confusion around start/end of the period. We can later fix the wording of the summary line. |
|
@techytushar Thanks a lot! |
|
I opened an issue here: #20324 |
Signed-off-by: Tushar Mittal [email protected]
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 --diffpython 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.