-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
CLN/DOC: Refactor timeseries.rst intro and overview #22728
CLN/DOC: Refactor timeseries.rst intro and overview #22728
Conversation
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.
Looks great, just added some ideas that I think would make the doc clearer.
doc/source/timeseries.rst
Outdated
saturday.day_name() | ||
# Add 1 business day (Friday --> Monday) | ||
monday = ts + pd.tseries.offsets.BDay() | ||
monday.day_name() |
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.
may be we could rename ts
to friday
? I think it'd make the example more consistent and easier to understand.
doc/source/timeseries.rst
Outdated
converted = ts.asfreq('45Min', method='pad') | ||
converted.head() | ||
idx = pd.date_range('2018-01-01', periods=72, freq='H') | ||
ts = pd.Series(np.random.randn(len(idx)), index=idx) |
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'd prefer range
over randn
. The data won't also be deterministic, but when you do the mean, it'll be more clear, and the means will increase.
Also, may be it's difficult, but if you can find a resampling example where the original is much smaller than 72, I think it'd help users if we show the original data before showing the transformed data.
doc/source/timeseries.rst
Outdated
@@ -1443,7 +1486,7 @@ time. The method for this is :meth:`~Series.shift`, which is available on all of | |||
the pandas objects. | |||
|
|||
.. ipython:: python | |||
|
|||
ts = pd.Series(np.random.randn(len(rng)), index=rng) |
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 as before regarding random
vs range
doc/source/timeseries.rst
Outdated
pd.Series(pd.date_range('2000', freq='D', periods=3)) | ||
|
||
:class:`Series` and :class:`DataFrame` have extended data type support and functionality for ``datetime`` and ``timedelta`` | ||
data when the time data is used as data itself. The other time related concepts will be stored as ``object`` data. |
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'd name "The other time related concepts", and may be show an example with a datetime Series, so it's easier to see what we exactly mean here.
Good suggestions @datapythonista. Incorporated all your comments. |
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.
Great work, looks perfect.
Thanks @datapythonista @WillAyd. |
* CLN/DOC: Refactor timeseries.rst intro and overview * Address review * Forgot missing is
* CLN/DOC: Refactor timeseries.rst intro and overview * Address review * Forgot missing is
Refactoring
timeseries.rst
introduction and overview to give a better introduction into pandas timeseries functionality:NaT
in the beginning