BUG: Ensure that we only cast if timezone is not None

Author: Phillip Cloud <cpcloud@gmail.com>

Closes #1147 from cpcloud/fix-tz and squashes the following commits:

236ead8 [Phillip Cloud] Add comment [ci skip]
4ac8f86 [Phillip Cloud] BUG: Ensure that we only cast if timezone is not None

ibis/expr/api.py

@@ -1884,7 +1884,15 @@ def between_time(arg, lower, upper, timezone=None):
     """
 
     if isinstance(arg.op(), _ops.Time):
-        arg = arg.op().args[0].cast(dt.Timestamp(timezone=timezone))
+        # Here we pull out the first argument to the underlying Time operation
+        # which is by definition (in _timestamp_value_methods) a
+        # TimestampValue. We do this so that we can potentially specialize the
+        # "between time" operation for timestamp_value_expr.time().between().
+        # A similar mechanism is triggered when creating expressions like
+        # t.column.distinct().count(), which is turned into t.column.nunique().
+        arg = arg.op().args[0]
+        if timezone is not None:
+            arg = arg.cast(dt.Timestamp(timezone=timezone))
         op = _ops.BetweenTime(arg, lower, upper)
     else:
         op = _ops.Between(arg, lower, upper)

ibis/pandas/tests/test_datetimelike.py

@@ -125,26 +125,32 @@ def test_times_ops(t, df):
 
 
 @pytest.mark.parametrize(
-    "tz, rconstruct",
-    [('US/Eastern', np.zeros),
-     ('UTC', np.ones),
-     (None, np.ones)])
+    'tz, rconstruct',
+    [
+        ('US/Eastern', np.zeros),
+        ('UTC', np.ones),
+        (None, np.ones)
+    ]
+)
+@pytest.mark.parametrize(
+    'column',
+    [
+        'plain_datetimes_utc',
+        'plain_datetimes_naive'
+    ]
+)
 @pytest.mark.skipif(PY2, reason="not enabled on PY2")
-def test_times_ops_with_tz(t, df, tz, rconstruct):
-    result = t.plain_datetimes_utc.time().between(
-        '01:00', '02:00', timezone=tz).execute()
+def test_times_ops_with_tz(t, df, tz, rconstruct, column):
     expected = rconstruct(len(df), dtype=bool)
-    tm.assert_numpy_array_equal(result, expected)
 
-    result = t.plain_datetimes_naive.time().between(
-        '01:00', '02:00', timezone=tz).execute()
-    expected = rconstruct(len(df), dtype=bool)
+    expr = t[column].time().between('01:00', '02:00', timezone=tz)
+    result = expr.execute()
     tm.assert_numpy_array_equal(result, expected)
 
-    # equivalence
-    expected = np.ones(len(df), dtype=bool)
-    ts = t.plain_datetimes_utc.cast(dt.Timestamp(timezone=tz))
-    result = ts.time().between('01:00', '02:00').execute()
+    # Test that casting behavior is the same as using the timezone kwarg
+    ts = t[column].cast(dt.Timestamp(timezone=tz))
+    expr = ts.time().between('01:00', '02:00')
+    result = expr.execute()
     tm.assert_numpy_array_equal(result, expected)