Create epoch extraction op #2178

xmnlab · 2020-04-07T19:10:20Z

Added epoch extraction operation to Clickhouse, CSV, Impala, MySQL, OmniSciDB, Pandas, Parquet, PostgreSQL, PySpark, SQLite and Spark

Extra information about epoch int32 vs int64:

datapythonista · 2020-04-13T13:59:47Z

ibis/pandas/execution/temporal.py

+    return pd.Series(
+        (pd.DatetimeIndex(data) - pd.Timestamp('1970-01-01'))
+        // pd.Timedelta('1s')
+    )


I think this is suboptimal. Not sure if pandas has a specific function to get the underlying data from a datetime, but at least this works, and should be faster:

return data.astype(int) // int(1e9)

thanks @datapythonista do you know if the result is the same? or could it have any round difference?

The internal representation is the number of nanoseconds since epoch. Didn't check, but I assume the .astype(int) is returning that internal representation without errors. I think the // will ignore second decimals, not round to the closer. But among both implementations, if there are differences, I'd go with my implementation, since it's taking the epoch directly, not with an intermediary delta. But I'd say it should be exactly the same.

ibis/tests/all/test_temporal.py

datapythonista · 2020-04-13T14:03:30Z

ibis/tests/all/test_temporal.py

+    if attr == 'epoch':
+        expected = pd.Series(
+            (pd.DatetimeIndex(df.timestamp_col) - pd.Timestamp('1970-01-01'))
+            // pd.Timedelta('1s')


Not sure if it's very useful to write a test with the same implementation as the tested function. If there is no better way to test this here, I'd remove epoch from this parametrization, and implement a separate test where you check that execute_epoch returns 10 for 1970-01-01 0:00:10 or something like that.

it is just the same implementation for pandas. so I think we should keep it here. and maybe add an extra test just for pandas. how does it sound to you?

Makes sense, didn't realize you'd be comparing with the results of other backends.

xmnlab · 2020-04-13T14:32:45Z

@datapythonista thanks for the review. the current PR will add epoch for all backends (as much as possible). I just need to stop working on that right now because I am investigating the CI problem. I hope to be back to this PR soon. thanks again for the review.

pep8speaks · 2020-05-30T02:11:06Z

Hello @xmnlab! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-06-29 15:39:28 UTC

xmnlab · 2020-05-31T00:23:22Z

@datapythonista do you know why it is not working on windows?

ps: the pandas version used is 1.0.4

ibis\tests\all\test_temporal.py:62: in test_timestamp_extract
    expected = df.timestamp_col.astype(int) // int(1e9)
c:\miniconda\envs\ibis36\lib\site-packages\pandas\core\generic.py:5698: in astype
    new_data = self._data.astype(dtype=dtype, copy=copy, errors=errors)
c:\miniconda\envs\ibis36\lib\site-packages\pandas\core\internals\managers.py:582: in astype
    return self.apply("astype", dtype=dtype, copy=copy, errors=errors)
c:\miniconda\envs\ibis36\lib\site-packages\pandas\core\internals\managers.py:442: in apply
    applied = getattr(b, f)(**kwargs)
c:\miniconda\envs\ibis36\lib\site-packages\pandas\core\internals\blocks.py:2223: in astype
    return super().astype(dtype=dtype, copy=copy, errors=errors)
c:\miniconda\envs\ibis36\lib\site-packages\pandas\core\internals\blocks.py:625: in astype
    values = astype_nansafe(vals1d, dtype, copy=True)
c:\miniconda\envs\ibis36\lib\site-packages\pandas\core\dtypes\cast.py:841: in astype_nansafe
    raise TypeError(f"cannot astype a datetimelike from [{arr.dtype}] to [{dtype}]")
E   TypeError: cannot astype a datetimelike from [datetime64[ns]] to [int32]

xmnlab · 2020-06-04T19:01:50Z

this PR is ready for review. thanks!

the error in LinuxTest py38_sql_parquet happened after tests succeed. as I reran that before .. probably it is related to that, according to the message error:

Information, ApplicationInsightsTelemetrySender correlated 2 events with X-TFS-Session 393403b6-d6fa-4c5c-a451-8ddc09a9b2e2
##[error]Artifact LinuxCondaEnvironment-38-main already exists for build 2565.

xmnlab · 2020-06-10T20:37:31Z

a friendly reminder about this PR. thanks!

xmnlab · 2020-06-12T22:57:43Z

rebased! ready for review again. thanks

datapythonista

lgtm, thanks @xmnlab

Added couple of suggestions, but looks ok as it is too.

docs/source/release/index.rst

datapythonista · 2020-06-19T12:54:41Z

ibis/clickhouse/operations.py

@@ -600,6 +605,7 @@ def _string_like(translator, expr):
    ops.ExtractDay: unary('toDayOfMonth'),
    ops.ExtractDayOfYear: unary('toDayOfYear'),
    ops.ExtractQuarter: unary('toQuarter'),
+    ops.ExtractEpoch: _extract_epoch,


Would make sense to directly use this? Feels like using one liners will make the code more readable, and it's simple enough IMO. Same would apply to other backends.

Suggested change

ops.ExtractEpoch: _extract_epoch,

ops.ExtractEpoch: lambda translator, expr: _call(translator, 'toRelativeSecondNum', *expr.op().args),

if it is not a big deal, I prefer to keep this. but if it would block this PR I can do it if no problem.

datapythonista · 2020-06-19T12:58:47Z

ibis/sql/postgres/compiler.py

-def _extract(fmt):
-    def translator(t, expr):
+def _extract(fmt, output_type=sa.SMALLINT):
+    def translator(t, expr, output_type=output_type):


This feels a bit weird. I'd remove this, I think it should still work:

Suggested change

def translator(t, expr, output_type=output_type):

def translator(t, expr):

actually I did it because I had a side effect without that.

Co-authored-by: Marc Garcia <garcia.marc@gmail.com>

xmnlab · 2020-06-21T23:28:32Z

thanks for the review @datapythonista, I applied your suggestion about the release note. thanks!

jreback

looks good, question on the api

jreback · 2020-06-26T21:03:49Z

ibis/expr/api.py

@@ -3425,6 +3426,7 @@ def _date_sub(left, right):
    day_of_week=_day_of_week,
    day_of_year=_extract_field('day_of_year', ops.ExtractDayOfYear),
    quarter=_extract_field('quarter', ops.ExtractQuarter),
+    epoch=_extract_field('epoch', ops.ExtractEpoch),


epoch_seconds or seconds_since_epoch would be a more descriptive name i think.

thanks @jreback , changed to epoch_seconds

xmnlab · 2020-07-01T17:18:58Z

@datapythonista @jreback the changes requested were applied. thanks!

jreback · 2020-07-09T02:53:23Z

thanks @xmnlab

datapythonista reviewed Apr 13, 2020

View reviewed changes

datapythonista added feature Features or general enhancements pandas The pandas backend labels Apr 13, 2020

xmnlab force-pushed the add-epoch-op branch from ef9afa7 to e46be29 Compare May 30, 2020 02:10

xmnlab force-pushed the add-epoch-op branch 2 times, most recently from 5768ab0 to 64f2d11 Compare May 30, 2020 22:34

xmnlab force-pushed the add-epoch-op branch from cb8825b to ec02249 Compare June 3, 2020 15:41

xmnlab marked this pull request as ready for review June 4, 2020 18:59

Create epoch extraction op

8125d78

xmnlab force-pushed the add-epoch-op branch from ec02249 to 8125d78 Compare June 12, 2020 20:02

datapythonista approved these changes Jun 19, 2020

View reviewed changes

Update docs/source/release/index.rst

b96676e

Co-authored-by: Marc Garcia <garcia.marc@gmail.com>

datapythonista assigned jreback Jun 26, 2020

jreback suggested changes Jun 26, 2020

View reviewed changes

Apply suggestion from review

42c6de0

jreback approved these changes Jul 9, 2020

View reviewed changes

jreback added this to the Next Feature Release milestone Jul 9, 2020

jreback merged commit 32b8380 into ibis-project:master Jul 9, 2020

xmnlab mentioned this pull request Jul 26, 2020

[IBIS] FEAT: Need to add Ibis support for various timestamp component operations in OmniSci Quansight/omnisci#56

Open

tswast mentioned this pull request Feb 4, 2021

feat: support difference calculation in timestamp aggregations GoogleCloudPlatform/professional-services-data-validator#195

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create epoch extraction op #2178

Create epoch extraction op #2178

xmnlab commented Apr 7, 2020 •

edited

Loading

datapythonista Apr 13, 2020

xmnlab Apr 13, 2020

datapythonista Apr 13, 2020

datapythonista Apr 13, 2020

xmnlab Apr 13, 2020

datapythonista Apr 13, 2020

xmnlab commented Apr 13, 2020

pep8speaks commented May 30, 2020 •

edited

Loading

xmnlab commented May 31, 2020 •

edited

Loading

xmnlab commented Jun 4, 2020

xmnlab commented Jun 10, 2020

xmnlab commented Jun 12, 2020

datapythonista left a comment

datapythonista Jun 19, 2020

xmnlab Jun 20, 2020

datapythonista Jun 19, 2020

xmnlab Jun 20, 2020

xmnlab commented Jun 21, 2020

jreback left a comment

jreback Jun 26, 2020

xmnlab Jun 29, 2020

xmnlab commented Jul 1, 2020

jreback commented Jul 9, 2020

	ops.ExtractEpoch: _extract_epoch,
	ops.ExtractEpoch: lambda translator, expr: _call(translator, 'toRelativeSecondNum', *expr.op().args),

	def translator(t, expr, output_type=output_type):
	def translator(t, expr):

Create epoch extraction op #2178

Create epoch extraction op #2178

Conversation

xmnlab commented Apr 7, 2020 • edited Loading

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

xmnlab commented Apr 13, 2020

pep8speaks commented May 30, 2020 • edited Loading

Comment last updated at 2020-06-29 15:39:28 UTC

xmnlab commented May 31, 2020 • edited Loading

xmnlab commented Jun 4, 2020

xmnlab commented Jun 10, 2020

xmnlab commented Jun 12, 2020

datapythonista left a 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

xmnlab commented Jun 21, 2020

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xmnlab commented Jul 1, 2020

jreback commented Jul 9, 2020

xmnlab commented Apr 7, 2020 •

edited

Loading

pep8speaks commented May 30, 2020 •

edited

Loading

xmnlab commented May 31, 2020 •

edited

Loading