-
Notifications
You must be signed in to change notification settings - Fork 590
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
docs: improve UDF signature docs #8194
Conversation
|
ACTION NEEDED Ibis follows the Conventional Commits specification for release automation. The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. |
|
the docs build error means you didn't check in (after rendering/previewing) the frozen execution outputs for the doc, so it's trying to run them |
|
It looks like we are running snowflake queries in this part of the docs so for now someone with credentials will need to run it. In the meantime we should ideally move the snowflake stuff to a different document or remove it. |
|
(not looking at the code as I say this) can we just switch the examples to duckdb or something the doesn't need creds? |
|
Does the rest of the changes look good though? |
|
I removed/ported the snowflake stuff. we cab revert that if you want, but I think this is an improvement. |
- use "signature" not "input types" - improve prose and emphasis.
Also added callout to optional args signature for builtin UDFs. Didn't for the other UDF implementations.
Co-authored-by: Phillip Cloud <417981+cpcloud@users.noreply.github.com>
- Using snowflake prevented me from running the example because I don't have the creds. - Most of that content I don't think was needed. - I moved the relevant part, with specifying the new name, into the duckdb section - Also cleaned up imports to minimize things
|
@NickCrews Nah, let's keep it, this is strictly an improvement! Thank you! |
Also added callout to optional args signature for builtin UDFs. Didn't for the other UDF implementations.
Fixes #8170 (comment)