raulcd opened a new issue, #50464:
URL: https://github.com/apache/arrow/issues/50464

   ### Describe the enhancement requested
   
   When dropping support for older pandas we discussed whether the following 
code could be simplified:
   
   
https://github.com/apache/arrow/blob/97b867856baef4f9d6778eadbe660a48a1adff8c/python/pyarrow/src/arrow/python/arrow_to_pandas.cc#L1286-L1320
   
   Link to description:
   https://github.com/apache/arrow/pull/50444#discussion_r3552035760
   
   It seems it could be with something like:
   ```diff
   $ 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


-- 
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]

Reply via email to