Copilot commented on code in PR #50409:
URL: https://github.com/apache/arrow/pull/50409#discussion_r3641663683
##########
python/pyarrow/lib.pyx:
##########
@@ -239,8 +239,5 @@ include "io.pxi"
# IPC / Messaging
include "ipc.pxi"
-# Micro-benchmark routines
-include "benchmark.pxi"
-
# Public API
include "public-api.pxi"
Review Comment:
The benchmark include was removed from the extension build, which drops the
previously importable pyarrow.benchmark shim / benchmark_PandasObjectIsNull
entry point and the associated ASV microbenchmark. This is a behavior/API
change that isn’t mentioned in the PR description.
##########
python/pyarrow/src/arrow/python/datetime.cc:
##########
@@ -67,8 +67,13 @@ static PyStructSequence_Field MonthDayNanoField[] = {
{nullptr, nullptr}};
static PyStructSequence_Desc MonthDayNanoTupleDesc = {
- NonConst("MonthDayNano"),
- NonConst("A calendar interval consisting of months, days and
nanoseconds."),
+ NonConst("pyarrow.lib.MonthDayNano"),
+ NonConst("A calendar interval consisting of months, days and
nanoseconds.\n"
Review Comment:
MonthDayNanoTupleDesc’s name change ("MonthDayNano" ->
"pyarrow.lib.MonthDayNano") affects the Python-level type name/repr (and can
affect pickling / doctests). This appears user-facing, but the PR description
states there are no user-facing changes.
##########
python/pyarrow/src/arrow/python/helpers.cc:
##########
@@ -89,15 +89,27 @@ Result<uint16_t> PyFloat_AsHalf(PyObject* obj) {
return PyArrayScalar_VAL(obj, Half);
} else {
return Status::TypeError("conversion to float16 expects a `float` or ",
- "`np.float16` object, got ",
Py_TYPE(obj)->tp_name);
+ "`np.float16` object, got ",
+ internal::PyObject_StdStringTypeName(obj));
}
}
namespace internal {
std::string PyBytes_AsStdString(PyObject* obj) {
ARROW_DCHECK(PyBytes_Check(obj));
- return std::string(PyBytes_AS_STRING(obj), PyBytes_GET_SIZE(obj));
+ char* buffer = nullptr;
+ Py_ssize_t length = 0;
+ PyBytes_AsStringAndSize(obj, &buffer, &length);
+ return std::string(buffer, length);
Review Comment:
PyBytes_AsStringAndSize() return value is ignored. In non-DCHECK builds this
can lead to constructing a std::string from a null buffer if a non-bytes object
is passed (or if the API fails), resulting in undefined behavior.
This issue also appears on line 108 of the same file.
--
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]
