-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deps: update to ibis-framework 9.x and newer sqlglot #827
Changes from all commits
b4e6a50
f1ce09d
3c0cae0
d224a52
28b6a31
bb68b2b
05550ac
d5622b6
3596edd
32c2ab6
5e0c1e7
d877261
3d63801
f8d2864
847f459
6a0bedc
9d56ee7
fc84cb8
bb2408f
de32335
e7dd60f
8672495
016a203
00163fe
129cfae
fdfc503
6ac497a
7c68e17
bbb7615
a4c49cd
11318e8
c0a5a5a
616c99f
f977cff
773d9e1
f266e34
7a03585
e1c7f5e
f3d6fed
387fbd9
502eb16
232f2f9
b9d6826
79c8f68
a44a745
d7089ca
133e053
ded2334
2edaa9d
8f6165f
da32b61
e423f89
46c6e25
cedc6fc
5016018
16f7878
6be3a38
90cae7c
82c0489
1c3fead
b3f52c9
c77120e
3d10584
fe7aa81
24aa3df
7ff77f3
db1e57f
5cc8597
dea7aed
00834c7
e8e5e97
822180a
6f054f4
c7167db
555453b
0762299
8d36966
4c90516
ad122af
ab84436
d425aa9
fa46553
4b3fb0f
f175596
f3a43b1
8ef190f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -842,7 +842,7 @@ def isin_op_impl(x: ibis_types.Value, op: ops.IsInOp): | |
@scalar_op_compiler.register_unary_op(ops.ToDatetimeOp, pass_op=True) | ||
def to_datetime_op_impl(x: ibis_types.Value, op: ops.ToDatetimeOp): | ||
if x.type() == ibis_dtypes.str: | ||
return vendored_ibis_ops.SafeCastToDatetime(x).to_expr() | ||
return x.try_cast(ibis_dtypes.Timestamp(None)) | ||
else: | ||
# Numerical inputs. | ||
if op.format: | ||
|
@@ -995,8 +995,14 @@ def eq_nulls_match_op( | |
y: ibis_types.Value, | ||
): | ||
"""Variant of eq_op where nulls match each other. Only use where dtypes are known to be same.""" | ||
left = x.cast(ibis_dtypes.str).fillna(ibis_types.literal("$NULL_SENTINEL$")) | ||
right = y.cast(ibis_dtypes.str).fillna(ibis_types.literal("$NULL_SENTINEL$")) | ||
literal = ibis_types.literal("$NULL_SENTINEL$") | ||
if hasattr(x, "fill_null"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is causing the fillna/fill_nbull divide? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ibis 9.0.0 -> Ibis 9.1.0. We need Ibis 9.0.0 for Python 3.9 support, at least until we vendor the ibis expr APIs too, which we'll want to do sometime between now and bigframes 2.0. |
||
left = x.cast(ibis_dtypes.str).fill_null(literal) | ||
right = y.cast(ibis_dtypes.str).fill_null(literal) | ||
else: | ||
left = x.cast(ibis_dtypes.str).fillna(literal) | ||
right = y.cast(ibis_dtypes.str).fillna(literal) | ||
|
||
return left == right | ||
|
||
|
||
|
@@ -1379,7 +1385,10 @@ def fillna_op( | |
x: ibis_types.Value, | ||
y: ibis_types.Value, | ||
): | ||
return x.fillna(typing.cast(ibis_types.Scalar, y)) | ||
if hasattr(x, "fill_null"): | ||
return x.fill_null(typing.cast(ibis_types.Scalar, y)) | ||
else: | ||
return x.fillna(typing.cast(ibis_types.Scalar, y)) | ||
|
||
|
||
@scalar_op_compiler.register_binary_op(ops.round_op) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -179,4 +179,8 @@ def value_to_join_key(value: ibis_types.Value): | |
"""Converts nullable values to non-null string SQL will not match null keys together - but pandas does.""" | ||
if not value.type().is_string(): | ||
value = value.cast(ibis_dtypes.str) | ||
return value.fillna(ibis_types.literal("$NULL_SENTINEL$")) | ||
return ( | ||
value.fill_null(ibis_types.literal("$NULL_SENTINEL$")) | ||
if hasattr(value, "fill_null") | ||
else value.fillna(ibis_types.literal("$NULL_SENTINEL$")) | ||
) | ||
Comment on lines
+182
to
+186
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is going on here? This seems fragile - what types no longer have fillna? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fillna was renamed to fill_null in ibis-project/ibis#9300 (ibis 9.1.0) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I do hate to have hasattr checks in our code. Should we just use fillna and suppress warnings? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI (no change necessary for this PR): An approximate quantile node type has been merged into ibis. ibis-project/ibis#9881
Once we start vendoring the Ibis expressions in addition to the compiler, we can take advantage of that (if we want).