Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 41 additions & 38 deletions ibis/backends/polars/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,32 +146,40 @@ def _cast(op, strict=True, **kw):
elif to.is_date():
if not strict:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this being overly restrictive? It looks like below the polars strptime API accepts a strict:bool arg, which makes me think we could support string try casting to dates and times and timestamps?

raise NotImplementedError(f"Unsupported try_cast to type: {to!r}")
if dtype.is_string():
return arg.str.strptime(pl.Date, "%Y-%m-%d")
elif to.is_timestamp():
if not strict:
raise NotImplementedError(f"Unsupported try_cast to type: {to!r}")

time_zone = to.timezone
time_unit = _TIMESTAMP_SCALE_TO_UNITS.get(to.scale, "us")

if dtype.is_numeric():
typ = pl.Datetime(time_unit="us", time_zone=time_zone)
time_unit = _TIMESTAMP_SCALE_TO_UNITS.get(to.scale, "us")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldnt all this stuff be inside the _cast_string_to_temporal()?

typ = pl.Datetime(time_unit="us", time_zone=to.timezone)
arg = (arg * 1_000_000).cast(typ)
if time_unit != "us":
arg = arg.dt.truncate(f"1{time_unit}")
return arg.alias(op.name)
elif dtype.is_string():
typ = pl.Datetime(time_unit=time_unit, time_zone=time_zone)
arg = arg.str.strptime(typ)
if time_unit == "s":
return arg.dt.truncate("1s")
return arg

if to.is_temporal() and dtype.is_string():
return _cast_string_to_temporal(arg, to, strict=strict)

typ = PolarsType.from_ibis(to)
return arg.cast(typ, strict=strict)


def _cast_string_to_temporal(arg, to, strict=True):
# polars doesn't support casting strings to temporal types
if to.is_date():
return arg.str.strptime(pl.Date, "%Y-%m-%d", strict=strict)
elif to.is_timestamp():
arg = arg.str.strptime(PolarsType.from_ibis(to), strict=strict)
# polars has no second-precision datetime type
if to.scale == 0:
return arg.dt.truncate("1s")
return arg
elif to.is_time():
return arg.str.strptime(pl.Time, "%H:%M:%S%.f", strict=strict)
raise com.OperationNotDefinedError(f"Unsupported cast from string to {to!r}")


@translate.register(ops.Field)
def column(op, **_):
return pl.col(op.name)
Expand Down Expand Up @@ -1099,17 +1107,15 @@ def array_collect(op, in_group_by=False, **kw):
@translate.register(ops.ArrayFlatten)
def array_flatten(op, **kw):
result = translate(op.arg, **kw)
return (
pl.when(result.is_null())
.then(None)
.when(result.list.len() == 0)
.then([])
# polars doesn't have an efficient API (yet?) for removing one level of
# nesting from an array so we use elementwise evaluation
#
# https://github.com/ibis-project/ibis/issues/10135
.otherwise(result.list.eval(pl.element().flatten()))
)
# polars doesn't have an efficient API (yet?) for removing one level of
# nesting from an array so we use elementwise evaluation
#
# https://github.com/ibis-project/ibis/issues/10135
#
# Null and empty inner arrays are filtered out before exploding, because
# `explode`'s handling of them differs between polars versions
element = pl.element()
return result.list.eval(element.filter(element.list.len() > 0).explode())


_date_methods = {
Expand Down Expand Up @@ -1173,10 +1179,9 @@ def extract_epoch_seconds(op, **kw):
@translate.register(ops.DayOfWeekName)
def day_of_week_name(op, **kw):
index = translate(op.arg, **kw).dt.weekday() - 1
arg = None
for i, name in enumerate(calendar.day_name):
arg = pl.when(index == i).then(pl.lit(name)).otherwise(arg)
return arg
return index.replace_strict(
dict(enumerate(calendar.day_name)), return_dtype=pl.String
)


@translate.register(ops.Unary)
Expand Down Expand Up @@ -1217,18 +1222,16 @@ def between(op, **kw):

dtype = PolarsType.from_ibis(arg_dtype)

lower_bound = op.lower_bound
lower = translate(lower_bound, **kw)

if lower_bound.dtype != arg_dtype:
lower = lower.cast(dtype)

upper_bound = op.upper_bound
upper = translate(upper_bound, **kw)

if upper_bound.dtype != arg_dtype:
upper = upper.cast(dtype)
def cast_bound(bound):
value = translate(bound, **kw)
if bound.dtype == arg_dtype:
return value
if bound.dtype.is_string() and arg_dtype.is_temporal():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love how we use polar's casting logic here, which is prone to drifting from the string->time casting we added in this pr in the def _cast(). Like why does that get a format string, and this doesn't? I would prefer if we had a singular def _cast_string_to_temporal(polars_str_expr, ibis_dest_type) and both callsites used it. But I haven't tried this, maybe this is more complex than it is worth.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NickCrews I gave it a shot in ac073e6, and after some guidance to keep things as aligned with the existing logic/behavior as possible, I think it pretty well matches what you suggsted.

return _cast_string_to_temporal(value, arg_dtype)
return value.cast(dtype)

lower = cast_bound(op.lower_bound)
upper = cast_bound(op.upper_bound)
return arg.is_between(lower, upper, closed="both")


Expand Down
16 changes: 16 additions & 0 deletions ibis/backends/polars/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,19 @@ def test_compile_with_memtable(con):
t = ibis.memtable({"a": [1, 2, 3], "b": [4, 5, 6]})
result = con.compile(t)
assert isinstance(result, pl.LazyFrame)


def test_cast_string_to_timestamp_seconds(con):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of these new tests should be in generic backend tests, not just for polars?

t = ibis.memtable({"s": ["2023-01-07 13:20:05.561"]})
result = con.execute(t.s.cast("timestamp(0)"))
assert result.iat[0] == pd.Timestamp("2023-01-07 13:20:05")


@pytest.mark.parametrize("to_type", ["timestamp", "timestamp(0)", "timestamp(9)"])
def test_between_string_timestamp_bounds(con, to_type):
# bounds should be parsed at the same precision as the column, so a value
# is always between itself
value = "2023-01-07 13:20:05.561000231"
t = ibis.memtable({"s": [value]})
result = con.execute(t.s.cast(to_type).between(value, value))
assert result.iat[0]
11 changes: 3 additions & 8 deletions ibis/backends/tests/test_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -1259,11 +1259,6 @@ def flatten_data():
reason="Arrays are never nullable",
raises=AssertionError,
),
pytest.mark.notyet(
["polars"],
reason="flattened empty arrays incorrectly insert a null",
raises=AssertionError,
),
],
),
],
Expand Down Expand Up @@ -1349,10 +1344,10 @@ def test_range_start_stop_step_zero(con, start, stop):
assert list(result) == []


@pytest.mark.notimpl(
["polars"],
@pytest.mark.xfail_version(
polars=["polars<2"],
raises=AssertionError,
reason="ibis hasn't implemented this behavior yet",
reason="explode turns empty lists into nulls",
)
@pytest.mark.notyet(
["flink"],
Expand Down
55 changes: 53 additions & 2 deletions ibis/backends/tests/test_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1854,6 +1854,32 @@ def hash_256(col):
param("float", "int", 0.0, 0, id="float_to_int"),
param("string", "int", "0", 0, id="string_to_int"),
param("string", "float", "0", 0.0, id="string_to_float"),
param(
"string",
"time",
"01:02:03",
datetime.time(1, 2, 3),
marks=[
pytest.mark.notimpl(
["sqlite"],
raises=AssertionError,
reason="SQLite returns Timedelta from execution",
),
pytest.mark.notyet(
["clickhouse", "impala"],
raises=AssertionError,
reason="backend doesn't have a time datatype",
),
pytest.mark.notyet(["druid"], raises=PyDruidProgrammingError),
pytest.mark.notyet(["exasol"], raises=ExaQueryError),
pytest.mark.notimpl(["pyspark"], raises=AttributeError),
pytest.mark.notyet(["oracle"], raises=OracleDatabaseError),
pytest.mark.notyet(
["athena"], raises=PyAthenaOperationalError, reason="no time type"
),
],
id="string_to_time",
),
param(
"array<int>",
"array<string>",
Expand Down Expand Up @@ -1915,6 +1941,24 @@ def test_cast(con, from_type, to_type, from_val, expected):
param(0.0, "int", 0),
param("0", "int", 0),
param("0.0", "float", 0.0),
param(
"01:02:03",
"time",
datetime.time(1, 2, 3),
marks=[
pytest.mark.notyet(
["clickhouse", "impala"],
raises=AssertionError,
reason="backend doesn't have a time datatype",
),
pytest.mark.notyet(["druid"], raises=PyDruidProgrammingError),
pytest.mark.notyet(["exasol"], raises=ExaQueryError),
pytest.mark.notimpl(["pyspark"], raises=AttributeError),
pytest.mark.notyet(
["athena"], raises=PyAthenaOperationalError, reason="no time type"
),
],
),
param(
datetime.datetime(2023, 1, 1),
"int",
Expand Down Expand Up @@ -1955,7 +1999,6 @@ def test_try_cast(con, from_val, to_type, expected):
"datafusion",
"druid",
"exasol",
"mysql",
"singlestoredb",
"oracle",
"postgres",
Expand All @@ -1971,11 +2014,19 @@ def test_try_cast(con, from_val, to_type, expected):
@pytest.mark.parametrize(
("from_val", "to_type"),
[
param("a", "int"),
param("a", "int", marks=pytest.mark.notimpl(["mysql"])),
param(
"a",
"time",
marks=pytest.mark.notyet(
["athena"], raises=PyAthenaOperationalError, reason="no time type"
),
),
param(
datetime.datetime(2023, 1, 1),
"int",
marks=[
pytest.mark.notimpl(["mysql"]),
pytest.mark.never(
["clickhouse", "pyspark", "flink", "databricks"],
reason="casts to 1672531200",
Expand Down
4 changes: 2 additions & 2 deletions requirements-dev.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 15 additions & 15 deletions uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading