Skip to content
This repository has been archived by the owner on Mar 29, 2023. It is now read-only.

Add timediff as another time operationr function #92

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

renato2099
Copy link
Contributor

closes #41

baed on https://cloud.google.com/bigquery/docs/reference/standard-sql/time_functions#time_diff and https://github.com/ibis-project/ibis-bigquery/pull/40/files#r620526603 I added a TIME_DIFF

hey @tswast I tried adding a unit test but it is not working :(
I am getting the following errors

FAILED tests/system/test_compiler.py::test_timestamp_timediff[h-<lambda>0] - AttributeError: 'TimeScalar' object has no attribute 'timediff'
FAILED tests/system/test_compiler.py::test_timestamp_timediff[h-<lambda>1] - AttributeError: k

any suggestions/advice please?

@saschahofmann
Copy link
Contributor

saschahofmann commented Sep 28, 2021

From what I can tell, bigquery's TIME_DIFF operation actually works differently than the standard ibis implementation.

class TimeDiff(BinaryOp):
    left = Arg(rlz.time)
    right = Arg(rlz.time)
    output_type = rlz.shape_like('left', dt.Interval('s'))

If I understand ibis correctly, this should always return something like TIME_DIFF(TIME left, TIME right, SECOND) ?

I don't think thats what your code is doing?

I don't think you can use _timestamp_op for this.

@saschahofmann
Copy link
Contributor

saschahofmann commented Sep 28, 2021

I just added the full TIMESTAMP_DIFF bigquery function to our project like this

class TimestampDiff(ValueOp):
    left = Arg(rlz.timestamp)
    right = Arg(rlz.timestamp)
    unit = Arg(rlz.string)
    output_type = rlz.shape_like("left", dt.float64)


def timestamp_diff(left, right, unit):
    return TimestampDiff(left, right, unit).to_expr()


TimestampValue.timestamp_diff = timestamp_diff


@compiles(TimestampDiff)
def _timestamp_diff(translator, expr):
    left, right, unit = expr.op().args
    t_left = translator.translate(left)
    t_right = translator.translate(right)

    # This part is specific to our project where the argument will be an ibis.literal string
    t_unit = _timestamp_units[translator.translate(unit).replace("'", "")]
    return f"TIMESTAMP_DIFF({t_left}, {t_right},  {t_unit})"

I am not sure whether we can do the same in this project since as I said above the standard ibis functionality works differently (I think).

@saschahofmann
Copy link
Contributor

I think to replace the standard ibis TimestampDiff operation (and the other to diffs) you could do something like this.

def _compile_timestamp_diff_op(op, bq_func, unit):
    def diff(translator, expr):
        left, right = expr.op().args
        t_left = translator.translate(left)
        t_right = translator.translate(right)

        return f"{bq_func}({t_left}, {t_right},  {unit})"

    return compiles(op)(diff)


_compile_timestamp_diff_op(TimestampDiff, "TIMESTAMP_DIFF", "SECOND")
_compile_timestamp_diff_op(TimeDiff, "TIME_DIFF", "SECOND")
_compile_timestamp_diff_op(DateDiff, "DATE_DIFF", "DAY")

Or just add them to the registry directly.

@jayceslesar
Copy link

jayceslesar commented Jan 28, 2022

I'm confused, what is the difference between this and calling a my_table.timestamp.sub(my_table.timestamp.lag(offset=1)) from the conventional ibis api?

@saschahofmann
Copy link
Contributor

saschahofmann commented Jan 29, 2022

Well have you tried compiling this with bigquery? At least at the time of writing few months ago I would get a ibis.common.exceptions.OperationNotDefinedError: No translation rule for <class 'ibis.expr.operations.TimestampDiff'> for that. Maybe this has been implemented now?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: add support for TimestampDiff operation
3 participants