rok commented on code in PR #50325:
URL: https://github.com/apache/arrow/pull/50325#discussion_r3722844124
##########
python/pyarrow/src/arrow/python/helpers.cc:
##########
@@ -338,24 +338,48 @@ struct ModuleOnceRunner {
static PyObject* uuid_UUID = nullptr;
static ModuleOnceRunner uuid_runner("uuid");
-} // namespace
-
-bool IsPyUuid(PyObject* obj) {
+PyObject* GetUuidClass() {
uuid_runner.RunOnce([](OwnedRef& module) {
OwnedRef ref;
if (ImportFromModule(module.obj(), "UUID", &ref).ok()) {
uuid_UUID = ref.obj();
}
});
- if (!uuid_UUID) return false;
- int result = PyObject_IsInstance(obj, uuid_UUID);
+ return uuid_UUID;
+}
+
+} // namespace
+
+bool IsPyUuid(PyObject* obj) {
+ PyObject* uuid_class = GetUuidClass();
+ if (!uuid_class) return false;
+ int result = PyObject_IsInstance(obj, uuid_class);
if (result < 0) {
PyErr_Clear();
return false;
}
return result != 0;
}
+Result<PyObject*> UuidFromBytes(std::string_view bytes) {
+ PyObject* uuid_class = GetUuidClass();
+ if (!uuid_class) {
+ return Status::Invalid("Could not import uuid.UUID");
+ }
+ OwnedRef py_bytes(
+ PyBytes_FromStringAndSize(bytes.data(),
static_cast<Py_ssize_t>(bytes.size())));
+ RETURN_IF_PYERROR();
+ OwnedRef kwargs(PyDict_New());
Review Comment:
This will create a new kwargs dictionary for every value in a uuid array
which is not really needed. Perhaps we can have the column visitor pass an
empty kwargs into `UuudFromBytes` to avoid this? See sketch below.
```diff
diff --git a/python/pyarrow/src/arrow/python/arrow_to_pandas.cc
b/python/pyarrow/src/arrow/python/arrow_to_pandas.cc
--- a/python/pyarrow/src/arrow/python/arrow_to_pandas.cc
+++ b/python/pyarrow/src/arrow/python/arrow_to_pandas.cc
@@ -1401,8 +1401,13 @@ struct ObjectWriterVisitor {
storage_arrays.push_back(extension_array.storage());
}
ChunkedArray storage(std::move(storage_arrays), type.storage_type());
+ OwnedRef args(PyTuple_New(0));
+ RETURN_IF_PYERROR();
+ OwnedRef kwargs(PyDict_New());
+ RETURN_IF_PYERROR();
auto WrapUuid = [&](const std::string_view& view, PyObject** out) {
- ARROW_ASSIGN_OR_RAISE(*out, internal::UuidFromBytes(view));
+ ARROW_ASSIGN_OR_RAISE(*out,
+ internal::UuidFromBytes(view, args.obj(),
kwargs.obj()));
return Status::OK();
};
return ConvertAsPyObjects<FixedSizeBinaryType>(options, storage,
WrapUuid,
diff --git a/python/pyarrow/src/arrow/python/helpers.cc
b/python/pyarrow/src/arrow/python/helpers.cc
--- a/python/pyarrow/src/arrow/python/helpers.cc
+++ b/python/pyarrow/src/arrow/python/helpers.cc
@@ -361,7 +361,8 @@ bool IsPyUuid(PyObject* obj) {
return result != 0;
}
-Result<PyObject*> UuidFromBytes(std::string_view bytes) {
+Result<PyObject*> UuidFromBytes(std::string_view bytes, PyObject* args,
+ PyObject* kwargs) {
PyObject* uuid_class = GetUuidClass();
if (!uuid_class) {
return Status::Invalid("Could not import uuid.UUID");
@@ -369,14 +370,10 @@ Result<PyObject*> UuidFromBytes(std::string_view
bytes) {
OwnedRef py_bytes(
PyBytes_FromStringAndSize(bytes.data(),
static_cast<Py_ssize_t>(bytes.size())));
RETURN_IF_PYERROR();
- OwnedRef kwargs(PyDict_New());
- RETURN_IF_PYERROR();
- if (PyDict_SetItemString(kwargs.obj(), "bytes", py_bytes.obj()) < 0) {
+ if (PyDict_SetItemString(kwargs, "bytes", py_bytes.obj()) < 0) {
RETURN_IF_PYERROR();
}
- OwnedRef args(PyTuple_New(0));
- PyObject* result = PyObject_Call(uuid_class, args.obj(), kwargs.obj());
+ PyObject* result = PyObject_Call(uuid_class, args, kwargs);
RETURN_IF_PYERROR();
return result;
}
diff --git a/python/pyarrow/src/arrow/python/helpers.h
b/python/pyarrow/src/arrow/python/helpers.h
--- a/python/pyarrow/src/arrow/python/helpers.h
+++ b/python/pyarrow/src/arrow/python/helpers.h
@@ -99,7 +99,8 @@ bool IsPyUuid(PyObject* obj);
// \brief Construct a uuid.UUID from 16 raw bytes
ARROW_PYTHON_EXPORT
-Result<PyObject*> UuidFromBytes(std::string_view bytes);
+Result<PyObject*> UuidFromBytes(std::string_view bytes, PyObject* args,
+ PyObject* kwargs);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
