rok commented on a change in pull request #10998:
URL: https://github.com/apache/arrow/pull/10998#discussion_r698647919
##########
File path: docs/source/cpp/compute.rst
##########
@@ -1101,19 +1101,29 @@ number of input and output types. The type to cast to
can be passed in a
:struct:`CastOptions` instance. As an alternative, the same service is
provided by a concrete function :func:`~arrow::compute::Cast`.
-+--------------------------+------------+--------------------+------------------+------------------------------+
-| Function name | Arity | Input types | Output type
| Options class |
-+==========================+============+====================+==================+==============================+
-| cast | Unary | Many | Variable
| :struct:`CastOptions` |
-+--------------------------+------------+--------------------+------------------+------------------------------+
-| strftime | Unary | Timestamp | String
| :struct:`StrftimeOptions` |
-+--------------------------+------------+--------------------+------------------+------------------------------+
-| strptime | Unary | String-like | Timestamp
| :struct:`StrptimeOptions` |
-+--------------------------+------------+--------------------+------------------+------------------------------+
++-----------------+------------+--------------------+------------------+------------------------------+-------+
+| Function name | Arity | Input types | Output type |
Options class | Notes |
++=================+============+====================+==================+==============================+=======+
+| cast | Unary | Many | Variable |
:struct:`CastOptions` | |
++-----------------+------------+--------------------+------------------+------------------------------+-------+
+| strftime | Unary | Timestamp | String |
:struct:`StrftimeOptions` | \(1) |
++-----------------+------------+--------------------+------------------+------------------------------+-------+
+| strptime | Unary | String-like | Timestamp |
:struct:`StrptimeOptions` | |
++-----------------+------------+--------------------+------------------+------------------------------+-------+
The conversions available with ``cast`` are listed below. In all cases, a
null input value is converted into a null output value.
+* \(1) Output precision of seconds (``%S``) flag depends on the input timestamp
+ precision. If the number of seconds can not be exactly represented with
seconds,
+ then the format is a decimal floating point number with a fixed format and a
+ precision matching that of the precision of the input. Note precision
increases
+ three decimal points points per step going from seconds to nanoseconds.
Review comment:
Thanks for catching that. I think what happened was an accidental force
push.
##########
File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc
##########
@@ -836,6 +841,10 @@ const FunctionDoc strftime_doc{
"Format timestamps according to a format string",
("For each input timestamp, emit a formatted string.\n"
"The time format string and locale can be set using StrftimeOptions.\n"
+ "Output precision of %S (seconds) flag depends on the input timestamp
precision. "
Review comment:
Updated. Could you please check if it's ok now. I'm not sure about the
convention.
##########
File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc
##########
@@ -836,6 +841,10 @@ const FunctionDoc strftime_doc{
"Format timestamps according to a format string",
("For each input timestamp, emit a formatted string.\n"
"The time format string and locale can be set using StrftimeOptions.\n"
+ "Output precision of %S (seconds) flag depends on the input timestamp
precision. "
Review comment:
This evaluates to true in MySQL on sqlfiddle:
```SQL
SELECT WEEK('2008-1-6', 1) != WEEK('2008-1-6', 4)
```
##########
File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc
##########
@@ -836,6 +841,10 @@ const FunctionDoc strftime_doc{
"Format timestamps according to a format string",
("For each input timestamp, emit a formatted string.\n"
"The time format string and locale can be set using StrftimeOptions.\n"
+ "Output precision of %S (seconds) flag depends on the input timestamp
precision. "
Review comment:
This evaluates to true in MySQL on sqlfiddle:
```SQL
SELECT WEEK('2008-1-6', 1) != WEEK('2008-1-6', 4)
```
Mode | First day of week | Range | Week 1 is the first week
-- | -- | -- | --
0 | Sunday | 0-53 | with a Sunday in this year
1 | Monday | 0-53 | with 4 or more days this year
2 | Sunday | 1-53 | with a Sunday in this year
3 | Monday | 1-53 | with 4 or more days this year
4 | Sunday | 0-53 | with 4 or more days this year
5 | Monday | 0-53 | with a Monday in this year
6 | Sunday | 1-53 | with 4 or more days this year
7 | Monday | 1-53 | with a Monday in this year
##########
File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc
##########
@@ -835,8 +840,12 @@ const FunctionDoc subsecond_doc{
const FunctionDoc strftime_doc{
"Format timestamps according to a format string",
("For each input timestamp, emit a formatted string.\n"
- "The time format string and locale can be set using StrftimeOptions.\n"
- "An error is returned if the timestamps don't have a defined timezone,\n"
+ "The time format string and locale can be set using StrftimeOptions. "
+ "Output precision of %S (seconds) flag depends on the input timestamp
precision. "
+ "Timestamps with second precision are represented as integers while
milliseconds, "
+ "microsecond and nanoseconds are represented as fixed floating point
numbers with "
+ "3, 6 and 9 decimal places respectively.\n"
+ "An error is returned if the timestamps don't have a defined timezone,"
Review comment:
It seems error description is outdated for all extract kernels. Thanks
for pointing it out! Fixing.
##########
File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc
##########
@@ -835,8 +840,12 @@ const FunctionDoc subsecond_doc{
const FunctionDoc strftime_doc{
"Format timestamps according to a format string",
("For each input timestamp, emit a formatted string.\n"
- "The time format string and locale can be set using StrftimeOptions.\n"
- "An error is returned if the timestamps don't have a defined timezone,\n"
+ "The time format string and locale can be set using StrftimeOptions. "
+ "Output precision of %S (seconds) flag depends on the input timestamp
precision. "
+ "Timestamps with second precision are represented as integers while
milliseconds, "
+ "microsecond and nanoseconds are represented as fixed floating point
numbers with "
+ "3, 6 and 9 decimal places respectively.\n"
Review comment:
Done.
##########
File path: docs/source/cpp/compute.rst
##########
@@ -1094,19 +1094,29 @@ number of input and output types. The type to cast to
can be passed in a
:struct:`CastOptions` instance. As an alternative, the same service is
provided by a concrete function :func:`~arrow::compute::Cast`.
-+--------------------------+------------+--------------------+------------------+------------------------------+
-| Function name | Arity | Input types | Output type
| Options class |
-+==========================+============+====================+==================+==============================+
-| cast | Unary | Many | Variable
| :struct:`CastOptions` |
-+--------------------------+------------+--------------------+------------------+------------------------------+
-| strftime | Unary | Timestamp | String
| :struct:`StrftimeOptions` |
-+--------------------------+------------+--------------------+------------------+------------------------------+
-| strptime | Unary | String-like | Timestamp
| :struct:`StrptimeOptions` |
-+--------------------------+------------+--------------------+------------------+------------------------------+
++-----------------+------------+--------------------+------------------+------------------------------+-------+
+| Function name | Arity | Input types | Output type |
Options class | Notes |
++=================+============+====================+==================+==============================+=======+
+| cast | Unary | Many | Variable |
:struct:`CastOptions` | |
++-----------------+------------+--------------------+------------------+------------------------------+-------+
+| strftime | Unary | Timestamp | String |
:struct:`StrftimeOptions` | \(1) |
++-----------------+------------+--------------------+------------------+------------------------------+-------+
+| strptime | Unary | String-like | Timestamp |
:struct:`StrptimeOptions` | |
++-----------------+------------+--------------------+------------------+------------------------------+-------+
The conversions available with ``cast`` are listed below. In all cases, a
null input value is converted into a null output value.
+* \(1) Output precision of seconds (``%S``) flag depends on the input timestamp
+ precision. If the number of seconds can not be exactly represented with
seconds,
Review comment:
Makes sense yeah. Done.
--
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]
