Skip to content

[Python] Simplify arrow_to_pandas DateOffset handling for nanoseconds/milliseconds #50464

Description

@raulcd

Describe the enhancement requested

When dropping support for older pandas we discussed whether the following code could be simplified:

template <typename Type>
enable_if_t<std::is_same<Type, MonthDayNanoIntervalType>::value, Status> Visit(
const Type& type) {
OwnedRef args(PyTuple_New(0));
OwnedRef kwargs(PyDict_New());
RETURN_IF_PYERROR();
auto to_date_offset = [&](const MonthDayNanoIntervalType::MonthDayNanos& interval,
PyObject** out) {
ARROW_DCHECK(internal::BorrowPandasDataOffsetType() != nullptr);
// DateOffset objects do not add nanoseconds component to pd.Timestamp.
// as of Pandas 1.3.3
// (https://github.com/pandas-dev/pandas/issues/43892).
// So convert microseconds and remainder to preserve data
// but give users more expected results.
int64_t microseconds = interval.nanoseconds / 1000;
int64_t nanoseconds;
if (interval.nanoseconds >= 0) {
nanoseconds = interval.nanoseconds % 1000;
} else {
nanoseconds = -((-interval.nanoseconds) % 1000);
}
PyDict_SetItemString(kwargs.obj(), "months", PyLong_FromLong(interval.months));
PyDict_SetItemString(kwargs.obj(), "days", PyLong_FromLong(interval.days));
PyDict_SetItemString(kwargs.obj(), "microseconds",
PyLong_FromLongLong(microseconds));
PyDict_SetItemString(kwargs.obj(), "nanoseconds", PyLong_FromLongLong(nanoseconds));
*out =
PyObject_Call(internal::BorrowPandasDataOffsetType(), args.obj(), kwargs.obj());
RETURN_IF_PYERROR();
return Status::OK();
};
return ConvertAsPyObjects<MonthDayNanoIntervalType>(options, data, to_date_offset,
out_values);
}

Link to description:
#50444 (comment)

It seems it could be with something like:

$ git diff
diff --git a/python/pyarrow/src/arrow/python/arrow_to_pandas.cc b/python/pyarrow/src/arrow/python/arrow_to_pandas.cc
index 348d352a04..581da12577 100644
--- a/python/pyarrow/src/arrow/python/arrow_to_pandas.cc
+++ b/python/pyarrow/src/arrow/python/arrow_to_pandas.cc
@@ -1292,24 +1292,10 @@ struct ObjectWriterVisitor {
     auto to_date_offset = [&](const MonthDayNanoIntervalType::MonthDayNanos& interval,
                               PyObject** out) {
       ARROW_DCHECK(internal::BorrowPandasDataOffsetType() != nullptr);
-      // DateOffset objects do not add nanoseconds component to pd.Timestamp.
-      // as of Pandas 1.3.3
-      // (https://github.com/pandas-dev/pandas/issues/43892).
-      // So convert microseconds and remainder to preserve data
-      // but give users more expected results.
-      int64_t microseconds = interval.nanoseconds / 1000;
-      int64_t nanoseconds;
-      if (interval.nanoseconds >= 0) {
-        nanoseconds = interval.nanoseconds % 1000;
-      } else {
-        nanoseconds = -((-interval.nanoseconds) % 1000);
-      }
 
       PyDict_SetItemString(kwargs.obj(), "months", PyLong_FromLong(interval.months));
       PyDict_SetItemString(kwargs.obj(), "days", PyLong_FromLong(interval.days));
-      PyDict_SetItemString(kwargs.obj(), "microseconds",
-                           PyLong_FromLongLong(microseconds));
-      PyDict_SetItemString(kwargs.obj(), "nanoseconds", PyLong_FromLongLong(nanoseconds));
+      PyDict_SetItemString(kwargs.obj(), "nanoseconds", PyLong_FromLongLong(interval.nanoseconds));
       *out =
           PyObject_Call(internal::BorrowPandasDataOffsetType(), args.obj(), kwargs.obj());
       RETURN_IF_PYERROR();

but as shared on the PR comment it might cause a UX change. Nowadays we don't roundtrip hours/minutes on DateOffset, we show microseconds and nanoseconds. With a change like the above we will be consistent and we would show, Month/Day/Nano instead of Month/Day/Microseconds/Nano.

This issue is to track this code simplifaction.

Component(s)

Python

Metadata

Metadata

Assignees

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions