DOC: add Raises, Examples and See Also sections to methods at_time/between_time/first/last #20725

topper-123 · 2018-04-17T20:08:51Z

Added note that methods at_time/between_time/first/last requires that the index is a DateTimeIndex.

Added functioning examples to the method doc strings.

Added See Also section to the mentioned method doc string.

EDIT: The doc strings for DatetimeIndex.indexer_at_time and DatetimeIndex.indexer_between_time said they return a timeseries, while in reality they return arrays. I've corrected that, and made some minor updates to those doc strings in addition.

topper-123 · 2018-04-17T21:03:37Z

The failures are in test_parquet and test_network, so unrelated to this PR.

jschendel · 2018-04-17T21:51:18Z

pandas/core/generic.py

@@ -6703,13 +6703,39 @@ def at_time(self, time, asof=False):
        """
        Select values at particular time of day (e.g. 9:30AM).

+        Notes
+        -----
+        For this method to work, the index must to be a :class:`DateTimeIndex`


The "T" should be lowercase in DatetimeIndex

Likewise for the other "Notes" sections

jschendel · 2018-04-17T21:53:16Z

pandas/core/generic.py

+        --------
+        between_time : Select values between particular times of the day
+        first : select initial periods of time series based on a date offset
+        last: select final periods of time series based on a date offset


The capitalization is inconsistent at the beginning of the descriptions: "S" vs "s" in select.

Likewise in the other "See Also" sections

codecov · 2018-04-18T06:08:08Z

Codecov Report

Merging #20725 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #20725      +/-   ##
==========================================
+ Coverage   91.81%   91.82%   +0.01%     
==========================================
  Files         153      153              
  Lines       49481    49483       +2     
==========================================
+ Hits        45430    45437       +7     
+ Misses       4051     4046       -5

Flag	Coverage Δ
#multiple	`90.22% <100%> (+0.01%)`	⬆️
#single	`41.84% <0%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/datetimes.py	`95.76% <ø> (ø)`	⬆️
pandas/core/generic.py	`96.12% <100%> (+0.14%)`	⬆️
pandas/util/testing.py	`84.59% <0%> (+0.2%)`	⬆️

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 bd4332f...18025cf. Read the comment docs.

topper-123 · 2018-04-18T06:08:53Z

Comments from @jschendel have been addressed, thanks for the review.

jreback · 2018-04-21T17:19:39Z

pandas/core/generic.py

@@ -6703,13 +6703,40 @@ def at_time(self, time, asof=False):
        """
        Select values at particular time of day (e.g. 9:30AM).



put this in a Raises section instead

jreback · 2018-04-21T17:19:42Z

pandas/core/generic.py

+        2018-04-09 12:00:00  2
+        2018-04-10 00:00:00  3
+        2018-04-10 12:00:00  4
+        >>> ts.at_time('12:00')


blank line between cases

jreback · 2018-04-21T17:20:08Z

pandas/core/generic.py

+        By setting ``start_time`` to be later than ``end_time``,
+        you can get the times that are *not* between the two times.
+
+        Notes


same as above

jreback · 2018-04-21T17:20:51Z

pandas/core/indexes/datetimes.py

+            datetime.time or string in appropriate format ("%H:%M", "%H%M",
+            "%I:%M%p", "%I%M%p", "%H:%M:%S", "%H%M%S", "%I:%M:%S%p",
+            "%I%M%S%p").
+            If  strings, then ``tseries.tools.to_time`` is used to convert to


this last sentence is refering to an internal method, remove it

jreback · 2018-04-21T17:21:05Z

pandas/core/indexes/datetimes.py


        Returns
        -------
-        values_at_time : TimeSeries
+        values_at_time : array


boolean array

Actually it returns an array of integers (i.e. integer location)

jreback · 2018-04-21T17:21:18Z

pandas/core/indexes/datetimes.py


        Parameters
        ----------
        start_time, end_time : datetime.time, str
            datetime.time or string in appropriate format ("%H:%M", "%H%M",
            "%I:%M%p", "%I%M%p", "%H:%M:%S", "%H%M%S", "%I:%M:%S%p",
-            "%I%M%S%p")
+            "%I%M%S%p").
+            If  strings, then ``tseries.tools.to_time`` is used to convert to


jreback · 2018-04-21T17:21:49Z

are these functions in api.rst?

topper-123 · 2018-04-21T17:31:14Z

are these functions in api.rst?

Yes for all of them.

topper-123 · 2018-04-21T18:49:23Z

Comments addessed.

jreback · 2018-04-21T22:38:18Z

pandas/core/generic.py

@@ -6703,13 +6703,42 @@ def at_time(self, time, asof=False):
        """
        Select values at particular time of day (e.g. 9:30AM).

+        Raises
+        -----
+        AttributeError


really? this should be something else, prob a TypeError

Changed to TypeError, and whatsnew has gotten a note on this.

jreback · 2018-04-21T22:39:08Z

pandas/core/generic.py

@@ -6985,17 +7054,46 @@ def first(self, offset):
        Convenience method for subsetting initial periods of time series data
        based on a date offset.

+        Raises
+        -----
+        NotImplementedError


should also be a TypeError

Oh, I read the code wrong here, it already raised TypeError. doc string changed

pep8speaks · 2018-04-22T10:17:46Z

Hello @topper-123! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on May 06, 2018 at 12:07 Hours UTC

topper-123 · 2018-04-22T10:20:39Z

Comments addressed.

topper-123 · 2018-04-25T14:02:49Z

Is this ok, @jreback?

jreback · 2018-05-04T10:37:50Z

pandas/core/generic.py

        """
        from pandas.tseries.frequencies import to_offset
        if not isinstance(self.index, DatetimeIndex):
-            raise NotImplementedError("'first' only supports a DatetimeIndex "
-                                      "index")
+            raise TypeError("'first' only supports a DatetimeIndex index")


can you add some tests that assert the TypeError? (was this not tested before)?

They were not there beforehand, so I've added.

A question: There exists test for Series.first/last, but not DataFrame.first/last. Should i copy the relevant tests from series/test_timeseries.py to frame/test_time_series.py, or is that not relevant?

sure that would be great (do a search for them as well, it maybe that they are elsewhere, but they should be in test_timeseries

topper-123 · 2018-05-06T13:24:27Z

I've added the tests to frame/test_timeseries.py as discussed above and everything is green.

jreback · 2018-05-07T10:29:53Z

if anyone has comment: @jorisvandenbossche @TomAugspurger @toobaz otherwise lgtm.

TomAugspurger · 2018-05-07T10:41:44Z

Looks good.

…

On Mon, May 7, 2018 at 5:29 AM, Jeff Reback ***@***.***> wrote: if anyone has comment: @jorisvandenbossche <https://github.com/jorisvandenbossche> @TomAugspurger <https://github.com/TomAugspurger> @toobaz <https://github.com/toobaz> otherwise lgtm. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#20725 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHItvrQkq8pTC54DXy-ewrRM4dyxOgks5twCImgaJpZM4TY95o> .

jreback · 2018-05-07T10:43:19Z

thank @topper-123

jschendel reviewed Apr 17, 2018

View reviewed changes

topper-123 force-pushed the time_methods_docs branch from bec54c3 to a101788 Compare April 18, 2018 06:07

topper-123 force-pushed the time_methods_docs branch 2 times, most recently from 81f6b51 to 2b063e9 Compare April 21, 2018 14:31

jreback requested changes Apr 21, 2018

View reviewed changes

jreback added the Docs label Apr 21, 2018

topper-123 force-pushed the time_methods_docs branch from 2b063e9 to 9472196 Compare April 21, 2018 18:40

topper-123 force-pushed the time_methods_docs branch from 9472196 to c470fdd Compare April 21, 2018 18:54

jreback requested changes Apr 21, 2018

View reviewed changes

topper-123 force-pushed the time_methods_docs branch 2 times, most recently from 9a32d80 to 3a6de78 Compare April 22, 2018 10:17

topper-123 force-pushed the time_methods_docs branch 2 times, most recently from 3a5ad92 to d482d3e Compare April 22, 2018 11:06

topper-123 force-pushed the time_methods_docs branch 5 times, most recently from 3cee9f2 to 8235074 Compare April 30, 2018 19:03

jreback requested changes May 4, 2018

View reviewed changes

jreback added the Datetime Datetime data dtype label May 4, 2018

topper-123 changed the title ~~DOC: add Notes, Examples and See Also to methods at_time/between_time/first/last~~ DOC: add Raises, Examples and See Also sections to methods at_time/between_time/first/last May 5, 2018

topper-123 force-pushed the time_methods_docs branch 2 times, most recently from 0aadb90 to 154e93f Compare May 5, 2018 18:19

tp added 2 commits May 5, 2018 21:57

add Notes, Examples and See Also to at_time/between_time/first/last

eb974ce

Improve doc strings, also indexer_at/between/_time

295323c

topper-123 force-pushed the time_methods_docs branch from 154e93f to e41cfe8 Compare May 5, 2018 20:58

Add tests for TypeError when index isn't a DatetimeIndex

18025cf

topper-123 force-pushed the time_methods_docs branch from e41cfe8 to 18025cf Compare May 6, 2018 12:06

jreback added this to the 0.23.0 milestone May 7, 2018

jreback approved these changes May 7, 2018

View reviewed changes

jreback merged commit 587a0dd into pandas-dev:master May 7, 2018

topper-123 deleted the time_methods_docs branch May 18, 2018 18:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: add Raises, Examples and See Also sections to methods at_time/between_time/first/last #20725

DOC: add Raises, Examples and See Also sections to methods at_time/between_time/first/last #20725

topper-123 commented Apr 17, 2018 •

edited

Loading

topper-123 commented Apr 17, 2018

jschendel Apr 17, 2018

jschendel Apr 17, 2018

jschendel Apr 17, 2018 •

edited

Loading

jschendel Apr 17, 2018

codecov bot commented Apr 18, 2018 •

edited

Loading

topper-123 commented Apr 18, 2018

jreback Apr 21, 2018

jreback Apr 21, 2018

jreback Apr 21, 2018

jreback Apr 21, 2018

jreback Apr 21, 2018

topper-123 Apr 21, 2018

jreback Apr 21, 2018

jreback commented Apr 21, 2018

topper-123 commented Apr 21, 2018

topper-123 commented Apr 21, 2018

jreback Apr 21, 2018

topper-123 Apr 22, 2018

jreback Apr 21, 2018

topper-123 Apr 22, 2018

pep8speaks commented Apr 22, 2018 •

edited

Loading

topper-123 commented Apr 22, 2018

topper-123 commented Apr 25, 2018

jreback May 4, 2018

topper-123 May 5, 2018

jreback May 5, 2018

topper-123 commented May 6, 2018

jreback commented May 7, 2018

TomAugspurger commented May 7, 2018 via email

jreback commented May 7, 2018

		@@ -6703,13 +6703,40 @@ def at_time(self, time, asof=False):
		"""
		Select values at particular time of day (e.g. 9:30AM).

DOC: add Raises, Examples and See Also sections to methods at_time/between_time/first/last #20725

DOC: add Raises, Examples and See Also sections to methods at_time/between_time/first/last #20725

Conversation

topper-123 commented Apr 17, 2018 • edited Loading

topper-123 commented Apr 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jschendel Apr 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 18, 2018 • edited Loading

Codecov Report

topper-123 commented Apr 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Apr 21, 2018

topper-123 commented Apr 21, 2018

topper-123 commented Apr 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Apr 22, 2018 • edited Loading

Comment last updated on May 06, 2018 at 12:07 Hours UTC

topper-123 commented Apr 22, 2018

topper-123 commented Apr 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topper-123 commented May 6, 2018

jreback commented May 7, 2018

TomAugspurger commented May 7, 2018 via email

jreback commented May 7, 2018

topper-123 commented Apr 17, 2018 •

edited

Loading

jschendel Apr 17, 2018 •

edited

Loading

codecov bot commented Apr 18, 2018 •

edited

Loading

pep8speaks commented Apr 22, 2018 •

edited

Loading