| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@kosiew Here is an alternate approach. Instead of relying on extension type features it is going to pass the Field information when creating the FFI array. This will capture pyarrow extensions as well as any other metadata that any user assigns on the input. I'm going to leave it in draft until I can finish up those additional items on my check list. What do you think? cc @paleolimbot |
Sorry, something went wrong.
Definitely! Passing the argument fields/return fields should do it. Using __arrow_c_schema__ might be more flexible than isinstance(x, pa.Field) (arro3, nanoarrow, and polars types would work too). We have a slightly different signature model in SedonaDB ("type matchers") because the existing signature matching doesn't consider metadata, but at the Arrow/FFI level we're doing approximately the same thing: apache/sedona-db#228 . We do use the concept of SedonaType for arguments and return type (but these are serializable to/deserializable from fields). |
Sorry, something went wrong.
| "_import_from_c", | ||
| ( | ||
| addr_of!(array) as Py_uintptr_t, | ||
| addr_of!(schema) as Py_uintptr_t, | ||
| ), |
There was a problem hiding this comment.
is the use of PyArrow's private _import_from_c advisable?
Sorry, something went wrong.
There was a problem hiding this comment.
This code is a near duplicate of how we already convert ArrayData into a pyarrow object. You can see the original here. The difference in this function is that we know the field instead of only the data type.
Sorry, something went wrong.
There was a problem hiding this comment.
A more modern way is to use __arrow_c_schema__ (although I think import_from_c will be around for a while). It's only a few lines:
https://github.com/apache/sedona-db/blob/main/python/sedonadb/src/import_from.rs#L151-L157
Sorry, something went wrong.
|
Also worth evaluating while we're doing this: For scalar values, is it possible for them to contain metadata? If I do pa.scalar(uuid.uuid4().bytes, type=pa.uuid()) and I check the type I should have the extension data. Maybe this is already supported, but as part of this PR I want to evaluate that as well. Opened new issue so there isn't too much scope creep |
Sorry, something went wrong.
There was a problem hiding this comment.
This PR enhances Python UDFs to support PyArrow Field information (including metadata and nullability) instead of only DataType information, enabling more sophisticated data type handling in Python-written scalar UDFs.
Changes:
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file| File | Description |
|---|---|
| src/udf.rs | Replaces create_udf with custom PythonFunctionScalarUDF implementation supporting Field-based metadata |
| src/lib.rs | Adds new array module to the crate |
| src/array.rs | Implements PyArrowArrayExportable for FFI conversion with Field information |
| python/tests/test_udf.py | Adds tests for UUID metadata handling and nullability preservation |
| python/datafusion/user_defined.py | Updates API to accept Field/DataType with helper conversion functions |
| pyproject.toml | Adds minimum PyArrow version constraint |
| docs/source/user-guide/common-operations/udf-and-udfa.rst | Documents Field vs DataType usage and references Rust UDF blog post |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Which issue does this PR close?
Closes #1172
Rationale for this change
Since we now have the ability to pass Field information instead of just DataType with ScalarUDFs, this feature adds similar support for Python written UDFs. Without this feature you must write your UDFs in rust and expose them to Python. This enhancement greatly expands the use cases where PyArrow data can be leveraged.
What changes are included in this PR?
Are there any user-facing changes?
This expands on the current API and is backwards compatible.