-
Notifications
You must be signed in to change notification settings - Fork 608
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
feat(api): expose the ability to refer to existing functions not defined in ibis APIs #7119
Conversation
91cb562
to
54398d8
Compare
a670a11
to
9ec2623
Compare
81de075
to
a4a02cd
Compare
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.
Excited to see this feature. Just a quick surface-level review for now.
5e4c8ac
to
68c97d9
Compare
68c97d9
to
0ba8913
Compare
|
||
|
||
@udf.scalar.builtin(name="arrayJaccardIndex") | ||
def array_jaccard_index(a: dt.Array[dt.int64], b: dt.Array[dt.int64]) -> float: |
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.
Datatype types should be the preferred spelling: dt.Array[dt.Int64]
, at least static type checkers will complain about the instances.
from ibis import udf | ||
|
||
@udf.scalar.builtin | ||
def mismatches(left: str, right: str) -> int: |
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.
This looks pretty nice!
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.
Do we need a note about annotations being optional? There is a separate PR for that.
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.
LGTM! This will be pretty useful!
This is super useful! Nice walkthrough writeup, that really is going to mean this gets used more. Thanks! |
This PR exposes our already-existing opaque UDFs (renamed to
builtin
here), and adds tests for the API.Draft mode until I add tests for more backends.