#21128 DOC: Add documentation for freq='infer' option of DatetimeIndex and TimedeltaIndex constructors #21201

kirakrishnan · 2018-05-25T04:31:07Z

closes DOC: Add documentation for freq='infer' option of DatetimeIndex and TimedeltaIndex constructors #21128
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

jschendel

Thanks @kirakrishnan, looking good! A few comments:

jschendel · 2018-05-25T07:21:37Z

doc/source/timedeltas.rst

   pd.TimedeltaIndex(['1 days', '1 days, 00:00:05',
                     np.timedelta64(2,'D'), datetime.timedelta(days=2,seconds=2)])

+   In [5]: pd.TimedeltaIndex(['0 days', '10 days', '20 days'])


I think you can remove this first example, since the existing example above is similar. I'm open to leaving this in though, if you'd rather keep this as a second example.

jschendel · 2018-05-25T07:28:15Z

doc/source/timedeltas.rst


+   In [5]: pd.TimedeltaIndex(['0 days', '10 days', '20 days'])
+   Out[5]: TimedeltaIndex(['0 days', '10 days', '20 days'], dtype='timedelta64[ns]', freq=None)
+


Can you add a sentence here to highlight the freq='infer' is being used here, and what it does, similar to the sentence added to the docstring.

jschendel · 2018-05-25T07:33:35Z

doc/source/timedeltas.rst

+   In [5]: pd.TimedeltaIndex(['0 days', '10 days', '20 days'])
+   Out[5]: TimedeltaIndex(['0 days', '10 days', '20 days'], dtype='timedelta64[ns]', freq=None)
+
+   In [4]: pd.TimedeltaIndex(['0 days', '10 days', '20 days'], freq='infer')


Can you put this in a .. ipython:: python block? A new block will need to be created if you add the sentence I suggested above. This essentially allows sphinx to generate the python output when the docs are being built. You'll only need to add the input line (without In [4]:) since the output will be generated. See the existing code sample a few lines above for an example of how to set this up.

jschendel · 2018-05-25T07:35:15Z

doc/source/timeseries.rst

    pd.Timestamp('2010/11/12')

+You can also use the `DatetimeIndex` constructor directly:
+In [3]: pd.DatetimeIndex(['2018-01-01', '2018-01-03', '2018-01-05'])


Can you put this in a .. ipython:: python block? (same procedure as in my previous comment)

jschendel · 2018-05-25T07:36:30Z

doc/source/timeseries.rst

+You can also use the `DatetimeIndex` constructor directly:
+In [3]: pd.DatetimeIndex(['2018-01-01', '2018-01-03', '2018-01-05'])
+Out[3]: DatetimeIndex(['2018-01-01', '2018-01-03', '2018-01-05'], dtype='datetime64[ns]', freq=None)
+


Can you add a sentence here to highlight the freq='infer' is being used here and modify the example below in a .. ipython:: python block? (similar sentence as described in my timedeltas.rst comment)

jschendel · 2018-05-25T07:37:57Z

pandas/core/indexes/datetimes.py

    freq : string or pandas offset object, optional
-        One of pandas date offset strings or corresponding objects
+        One of pandas date offset strings or corresponding objects. The string
+        'infer' can be passed in order to allow users to set the frequency of


I think this would be a little more direct if the "to allow users" is removed, so it just reads as "...in order to set the frequency...".

jschendel · 2018-05-25T07:38:15Z

pandas/core/indexes/timedeltas.py

-    freq: a frequency for the index, optional
+    freq : string or pandas offset object, optional
+        One of pandas date offset strings or corresponding objects. The string
+        'infer' can be passed in order to allow users to set the frequency of


same regarding "to allow users"

to keep my local updated

kirakrishnan · 2018-05-25T23:24:52Z

Hi @jschendel , I made the changes you suggested and committed it. when I try to create pull request I am getting this message Able to merge. These branches can be automatically merged. . Please tell me if I can leave it or Do I need to do something else to merge the changes?

codecov · 2018-05-26T00:59:38Z

Codecov Report

Merging #21201 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #21201   +/-   ##
=======================================
  Coverage   91.84%   91.84%           
=======================================
  Files         153      153           
  Lines       49505    49505           
=======================================
  Hits        45466    45466           
  Misses       4039     4039

Flag	Coverage Δ
#multiple	`90.23% <ø> (ø)`	⬆️
#single	`41.88% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/indexes/timedeltas.py	`91.24% <ø> (ø)`	⬆️
pandas/core/indexes/datetimes.py	`95.78% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c2844a...e1a80eb. Read the comment docs.

local update

jreback · 2018-05-29T00:32:11Z

lgtm.

should have a comment about actually setting the freq attribute as well?

e.g.

dti.freq = 'D' or whatever

jschendel · 2018-05-29T23:45:22Z

should have a comment about actually setting the freq attribute as well?

Yes, but I'll create a separate issue for that with some more specific details. Would be nice to update the freq docstring in addition to timeseries.rst and timedeltas.rst.

jschendel

Looks good! A few very minor things but should be ready to merge afterwards.

jschendel · 2018-05-29T23:50:25Z

doc/source/timedeltas.rst

                     np.timedelta64(2,'D'), datetime.timedelta(days=2,seconds=2)])

+'infer' can be passed in order to set the frequency of the index as the inferred frequency
+upon creation


Can you add a colon after creation, i.e. "...upon creation:"

jschendel · 2018-05-29T23:51:15Z

doc/source/timeseries.rst

    pd.Timestamp('2010/11/12')

+You can also use the `DatetimeIndex` constructor directly:
+


Can you put the first DatetimeIndex example here (the one without freq='infer')?

jschendel · 2018-05-29T23:51:54Z

doc/source/timeseries.rst

+You can also use the `DatetimeIndex` constructor directly:
+
+'infer' can be passed in order to set the frequency of the index as the inferred frequency
+upon creation


Can you add a colon after creation, i.e. "...upon creation:"

jschendel · 2018-05-29T23:53:59Z

pandas/core/indexes/timedeltas.py

+    freq : string or pandas offset object, optional
+        One of pandas date offset strings or corresponding objects. The string
+        'infer' can be passed in order to set the frequency of the index as the
+         inferred frequency upon creation


Looks like this line has one extra space at the beginning. The "i" in inferred should be aligned with the single quote on the line above. Not sure if this impacts how the output will be rendered though.

jreback · 2018-06-04T21:34:56Z

@kirakrishnan if you can update

jreback · 2018-06-19T01:20:48Z

@jschendel if you wouldn't mind updating, this looks ok otherwise. (pls merge on green as well).

jschendel · 2018-06-20T22:26:38Z

@jreback : it doesn't look like I have push access to update this (unless I'm missing something?), so will close and reboot as a new PR in a little bit.

jschendel · 2018-06-20T23:28:40Z

Closing in favor of #21566

vak added 4 commits May 25, 2018 00:15

#21128 adding documentation for freq in datetimes.py

16452fc

#21128 adding documentation for freq in timedeltas.py

3180573

#21128 adding examples for freq=infer field

efbd4d1

#21128 adding examples for freq=infer in timeseries.rst

aec1037

kirakrishnan mentioned this pull request May 25, 2018

DOC: Add documentation for freq='infer' option of DatetimeIndex and TimedeltaIndex constructors #21128

Closed

jschendel reviewed May 25, 2018

View reviewed changes

vak added 5 commits May 25, 2018 19:02

21128 updating infer documentation in timedeltas.rst

daf1f86

21128 updating infer documentation in timeseries.rst

e50abd3

21128 updating documentation for infer in datetimes.py

35164d5

21128 updating documentation for infer in timedeltas.py

93163a6

Merge branch 'master' of github.com:pandas-dev/pandas

38530b5

to keep my local updated

Merge branch 'master' of github.com:pandas-dev/pandas

e1a80eb

local update

jreback added Docs Datetime Datetime data dtype Timedelta Timedelta data type Frequency DateOffsets labels May 29, 2018

jreback added this to the 0.23.1 milestone May 29, 2018

jreback added the Needs Backport label May 29, 2018

jschendel reviewed May 29, 2018

View reviewed changes

jreback removed this from the 0.23.1 milestone Jun 4, 2018

jreback added this to the 0.24.0 milestone Jun 19, 2018

jschendel removed the Needs Backport label Jun 20, 2018

jschendel mentioned this pull request Jun 20, 2018

DOC: Add documentation for freq='infer' option of DatetimeIndex and TimedeltaIndex constructors #21566

Merged

2 tasks

jschendel closed this Jun 20, 2018


		In [5]: pd.TimedeltaIndex(['0 days', '10 days', '20 days'])
		Out[5]: TimedeltaIndex(['0 days', '10 days', '20 days'], dtype='timedelta64[ns]', freq=None)

		pd.Timestamp('2010/11/12')

		You can also use the `DatetimeIndex` constructor directly:

Uh oh!

#21128 DOC: Add documentation for freq='infer' option of DatetimeIndex and TimedeltaIndex constructors #21201

#21128 DOC: Add documentation for freq='infer' option of DatetimeIndex and TimedeltaIndex constructors #21201

Uh oh!

Conversation

kirakrishnan commented May 25, 2018 • edited by jschendel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jschendel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirakrishnan commented May 25, 2018

Uh oh!

codecov bot commented May 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jreback commented May 29, 2018

Uh oh!

jschendel commented May 29, 2018

Uh oh!

jschendel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented Jun 4, 2018

Uh oh!

jreback commented Jun 19, 2018

Uh oh!

jschendel commented Jun 20, 2018

Uh oh!

jschendel commented Jun 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kirakrishnan commented May 25, 2018 •

edited by jschendel

Loading

codecov bot commented May 26, 2018 •

edited

Loading