EnricoMi commented on code in PR #48466:
URL: https://github.com/apache/arrow/pull/48466#discussion_r2664429167
##########
python/pyarrow/src/arrow/python/arrow_to_pandas.cc:
##########
@@ -1618,7 +1630,14 @@ class DatetimeMilliWriter : public
DatetimeWriter<TimeUnit::MILLI> {
// Convert from days since epoch to datetime64[ms]
ConvertDatetime<int32_t, 86400000L>(*data, out_values);
} else if (type == Type::DATE64) {
- ConvertNumericNullable<int64_t>(*data, kPandasTimestampNull, out_values);
+ // Date64Type is millisecond timestamp
+ if (this->options_.truncate_date64_time) {
+ // Truncate intraday milliseconds
+ ConvertDatetimeWithTruncation<1L>(*data, out_values);
Review Comment:
Can we avoid computing the `... * 1L` for each value in the array when
`SHIFT == 1`? Or will the compiler optimize this away?
##########
python/pyarrow/src/arrow/python/arrow_to_pandas.cc:
##########
@@ -1548,6 +1547,19 @@ void ConvertDatesShift(const ChunkedArray& data,
int64_t* out_values) {
}
}
+template <int64_t SHIFT>
+inline void ConvertDatetimeWithTruncation(const ChunkedArray& data, int64_t*
out_values) {
+ for (int c = 0; c < data.num_chunks(); c++) {
+ const auto& arr = *data.chunk(c);
+ const int64_t* in_values = GetPrimitiveValues<int64_t>(arr);
+ for (int64_t i = 0; i < arr.length(); ++i) {
+ *out_values++ = arr.IsNull(i)
+ ? kPandasTimestampNull
+ : ((in_values[i] - in_values[i] %
kMillisecondsInDay) * SHIFT);
+ }
+ }
+}
Review Comment:
The `SHIFT` sounds like we are bit-shifting, where this is more a factor.
```suggestion
template <int64_t FACTOR>
inline void ConvertDatetimeWithTruncation(const ChunkedArray& data, int64_t*
out_values) {
for (int c = 0; c < data.num_chunks(); c++) {
const auto& arr = *data.chunk(c);
const int64_t* in_values = GetPrimitiveValues<int64_t>(arr);
for (int64_t i = 0; i < arr.length(); ++i) {
*out_values++ = arr.IsNull(i)
? kPandasTimestampNull
: ((in_values[i] - in_values[i] %
kMillisecondsInDay) * FACTOR);
}
}
}
```
##########
python/pyarrow/src/arrow/python/arrow_to_pandas.cc:
##########
@@ -1548,6 +1547,19 @@ void ConvertDatesShift(const ChunkedArray& data,
int64_t* out_values) {
}
}
+template <int64_t SHIFT>
+inline void ConvertDatetimeWithTruncation(const ChunkedArray& data, int64_t*
out_values) {
+ for (int c = 0; c < data.num_chunks(); c++) {
+ const auto& arr = *data.chunk(c);
+ const int64_t* in_values = GetPrimitiveValues<int64_t>(arr);
+ for (int64_t i = 0; i < arr.length(); ++i) {
+ *out_values++ = arr.IsNull(i)
+ ? kPandasTimestampNull
+ : ((in_values[i] - in_values[i] %
kMillisecondsInDay) * SHIFT);
+ }
+ }
+}
Review Comment:
Looks like this naming exists in `ConvertDatetime` as well :-(.
--
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]
