ERR: between_time now checks for argument types #11832

rockg · 2015-12-12T22:27:36Z

jreback · 2015-12-13T20:33:07Z

do you think passing non-time like strings are also an error?

e.g.
2015-01-01 09:00 would be valid (previously and after this one). We could restrict to Timedelta parsables I think (not exactly the same thing but would work).

rockg · 2015-12-13T22:49:12Z

Unfortunately, I don't know how to check that. How it currently works is the dateutil parser is used to parse the time string but a datetime.datetime is returned regardless if it's a time string or date string.

>>> from dateutil.parser import parse
>>> parse("2015-12-13 10:10")
datetime.datetime(2015, 12, 13, 10, 10)
>>> parse("10:10")
datetime.datetime(2015, 12, 13, 10, 10)
>>>

rockg · 2015-12-13T22:57:30Z

I see what your idea is but I'm just not sure that the Timedelta and dateutil parser are completely the same.

>>> Timedelta("10:10")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pandas/tslib.pyx", line 2277, in pandas.tslib.Timedelta.__new__ (pandas/tslib.c:41085)
    value = np.timedelta64(parse_timedelta_string(value, False))
  File "pandas/tslib.pyx", line 2862, in pandas.tslib.parse_timedelta_string (pandas/tslib.c:49601)
    raise ValueError("expected hh:mm:ss format")
ValueError: expected hh:mm:ss format

jreback · 2015-12-13T23:10:03Z

the timedelta parser is a bit strick for this, can prob make a mini one by doing something like:

try:
    return time.strptime(s, '%H:%M:%S')
except:
    try:
        return time.strptime(s, '%H:%M')
    except:
        # rise here

rockg · 2015-12-13T23:23:10Z

Seems like we should also do a milliseconds parse for safe measure.

>>> parse("10:10:10.123")
datetime.datetime(2015, 12, 13, 10, 10, 10, 123000)

jreback · 2015-12-13T23:24:30Z

don't use dateutil.parse we want a time only

rockg · 2015-12-13T23:25:42Z

My only comment was that it's currently supported so we need that additionally.

jreback · 2015-12-13T23:25:47Z

easiest might be to loop thru a list of allowed formats until u match (or exhaust the list and raise)

using strptime is strict (which is good)

rockg · 2015-12-20T12:57:57Z

I'm confused as to why this fails on 2.6. I created a 2.6 environment and the test works fine there.

Python 2.6.9 |Continuum Analytics, Inc.| (unknown, Aug 21 2014, 18:28:52) 
...
In [1]: from datetime import datetime

In [2]: datetime.strptime("2:00am", "%I:%M%p").time()
Out[2]: datetime.time(2, 0)

rockg · 2015-12-20T15:02:45Z

Ah, it's because that test uses the am/pm from it_IT locale. Skipped if there is a locale present.

jreback · 2015-12-21T14:20:55Z

pandas/tseries/index.py

@@ -1769,13 +1782,34 @@ def indexer_between_time(self, start_time, end_time, include_start=True,
        -------
        values_between_time : TimeSeries
        """
-        from dateutil.parser import parse
+        def _parse_time_string(time_object):
+


can u factor this routine out and out it in tseries/tools.py (eg next to the datetime parsing ones)

thrn just call it from here (u can also move the these to the corresponding testing module)

jreback · 2015-12-26T00:33:13Z

pandas/tseries/tools.py

+    arg : compat.string_types
+    format : str, list-like, default None
+        Format used to convert arg into a time object.  If None, default formats
+        are used.  To add an additional format use 


why are we allowing 'adding' time formats? your list seems like enough, no?

More for backwards compatibility. Our formats are a subset of dateutil's so want to ensure that the code still works for those that may have used a different format historically.

jreback · 2015-12-30T15:40:55Z

pandas/tseries/tools.py

+            return _convert_listlike(arg, format)
+
+        return _convert_listlike(np.array([arg]), format)[0]
+    except:


don't really like these blanket excepts. What hits this? why is it necessary. Rather catch these things inside _convert_listlike on a specific basis

It's to cover the "ignore" case for casting errors in which case arg is returned. I did it here as things are cast to a numpy array and so your original argument really isn't returned. However, for now I can remove that casting and then check for "ignore" when the casting fails and return the unmolested arg at that point.

Or I can just catch the ValueError and return arg at that point which might make more sense.

first soln is better - then if an uncaught error it will show up

The way it is currently written it will raise any exceptions that occur within _convert_listlike so you won't have any uncaught errors.

rockg · 2016-01-02T12:41:05Z

Hopefully we are all good here.

jreback · 2016-01-02T22:43:36Z

doc/source/whatsnew/v0.18.0.txt

@@ -205,7 +205,8 @@ other anchored offsets like ``MonthBegin`` and ``YearBegin``.
 Other API Changes
 ^^^^^^^^^^^^^^^^^

-
+- DataFrame between_time now only parses a fixed set of time strings.  Parsing


use double backticks DataFrame.between_time (though this also applies to Series, yes? I would put a mini-example here as well.

jreback · 2016-01-02T22:44:37Z

pandas/tseries/index.py

-        start_time : datetime.time or string
-        end_time : datetime.time or string
+        start_time, end_time : datetime.time or string
+            Time or string in appropriate format (e.g., "%H:%M", "%I%M%p")


datetime.time or string in appropriate time-like format (and include the formats you have above)

jreback · 2016-01-02T22:48:51Z

small comments.

pls run: git diff master | flake8 --diff and fix the PEP8 issues (this is going to become standard shortly).

this looks really good. I think we could expose this via pd.to_time. Though I we do need better time support. Pls create an issue so we can consider this in the future.

ping when green.

rockg · 2016-01-03T03:48:10Z

@jreback Green. I feel more comfortable leaving the ability to add another format given that we do not know how much people were relying on dateutil's parser. It's two lines of code and innocuous.

jreback · 2016-01-03T16:26:10Z

merged via 9c71dbf

thanks.

jreback added Datetime Datetime data dtype Error Reporting Incorrect or improved errors from pandas labels Dec 13, 2015

jreback added this to the 0.18.0 milestone Dec 13, 2015

rockg force-pushed the between_time-exception branch 2 times, most recently from 09bfda3 to 2fef431 Compare December 20, 2015 12:37

rockg force-pushed the between_time-exception branch from 2fef431 to b42e96b Compare December 20, 2015 14:43

jreback reviewed Dec 21, 2015
View reviewed changes

rockg force-pushed the between_time-exception branch from b42e96b to 4009405 Compare December 22, 2015 01:59

jreback reviewed Dec 26, 2015
View reviewed changes

rockg force-pushed the between_time-exception branch 2 times, most recently from de83d7c to c764bbd Compare December 30, 2015 14:24

jreback reviewed Dec 30, 2015
View reviewed changes

rockg force-pushed the between_time-exception branch 2 times, most recently from 4b26ac9 to e480401 Compare December 31, 2015 01:36

jreback reviewed Jan 2, 2016
View reviewed changes

rockg force-pushed the between_time-exception branch 2 times, most recently from 33cc8f2 to eb827c4 Compare January 3, 2016 02:33

ERR/ENH: between_time checks argument types and new to_time function

eadc308

rockg force-pushed the between_time-exception branch from eb827c4 to eadc308 Compare January 3, 2016 03:26

jreback closed this Jan 3, 2016

This was referenced Jan 3, 2016

ERR: between_time should raise on non-timelike objects #11818

Closed

API: expose pd.to_time #11947

Closed

jreback mentioned this pull request Mar 21, 2016

Issue passing a Timestamp to Series.between_time() #12680

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ERR: between_time now checks for argument types #11832

ERR: between_time now checks for argument types #11832

rockg commented Dec 12, 2015

jreback commented Dec 13, 2015

rockg commented Dec 13, 2015

rockg commented Dec 13, 2015

jreback commented Dec 13, 2015

rockg commented Dec 13, 2015

jreback commented Dec 13, 2015

rockg commented Dec 13, 2015

jreback commented Dec 13, 2015

rockg commented Dec 20, 2015

rockg commented Dec 20, 2015

jreback Dec 21, 2015

jreback Dec 26, 2015

rockg Dec 26, 2015

jreback Dec 30, 2015

rockg Dec 30, 2015

rockg Dec 30, 2015

jreback Dec 30, 2015

rockg Dec 30, 2015

rockg commented Jan 2, 2016

jreback Jan 2, 2016

jreback Jan 2, 2016

jreback commented Jan 2, 2016

rockg commented Jan 3, 2016

jreback commented Jan 3, 2016

ERR: between_time now checks for argument types #11832

ERR: between_time now checks for argument types #11832

Conversation

rockg commented Dec 12, 2015

jreback commented Dec 13, 2015

rockg commented Dec 13, 2015

rockg commented Dec 13, 2015

jreback commented Dec 13, 2015

rockg commented Dec 13, 2015

jreback commented Dec 13, 2015

rockg commented Dec 13, 2015

jreback commented Dec 13, 2015

rockg commented Dec 20, 2015

rockg commented Dec 20, 2015

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

Choose a reason for hiding this comment

rockg commented Jan 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jan 2, 2016

rockg commented Jan 3, 2016

jreback commented Jan 3, 2016