jorisvandenbossche commented on a change in pull request #10598:
URL: https://github.com/apache/arrow/pull/10598#discussion_r664692604



##########
File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc
##########
@@ -451,9 +524,15 @@ const FunctionDoc day_doc{
 
 const FunctionDoc day_of_week_doc{
     "Extract day of the week number",
-    ("Week starts on Monday denoted by 0 and ends on Sunday denoted by 6.\n"
+    ("By default, the week starts on Monday represented by 0 and ends on 
Sunday "
+     "represented "
+     "by 6.\n"

Review comment:
       ```suggestion
        "represented by 6.\n"
   ```

##########
File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc
##########
@@ -35,6 +36,7 @@ using arrow_vendored::date::days;
 using arrow_vendored::date::floor;
 using arrow_vendored::date::hh_mm_ss;
 using arrow_vendored::date::sys_time;
+using arrow_vendored::date::time_zone;

Review comment:
       This doesn't need to be added here?

##########
File path: docs/source/cpp/compute.rst
##########
@@ -988,44 +988,46 @@ Temporal component extraction
 These functions extract datetime components (year, month, day, etc) from 
timestamp type.
 Note: this is currently not supported for timestamps with timezone information.
 
-+--------------------+------------+-------------------+---------------+--------+
-| Function name      | Arity      | Input types       | Output type   | Notes  
|
-+====================+============+===================+===============+========+
-| year               | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| month              | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| day                | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| day_of_week        | Unary      | Temporal          | Int64         | \(1)   
|
-+--------------------+------------+-------------------+---------------+--------+
-| day_of_year        | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| iso_year           | Unary      | Temporal          | Int64         | \(2)   
|
-+--------------------+------------+-------------------+---------------+--------+
-| iso_week           | Unary      | Temporal          | Int64         | \(2)   
|
-+--------------------+------------+-------------------+---------------+--------+
-| iso_calendar       | Unary      | Temporal          | Struct        | \(3)   
|
-+--------------------+------------+-------------------+---------------+--------+
-| quarter            | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| hour               | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| minute             | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| second             | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| millisecond        | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| microsecond        | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| nanosecond         | Unary      | Temporal          | Int64         |        
|
-+--------------------+------------+-------------------+---------------+--------+
-| subsecond          | Unary      | Temporal          | Double        |        
|
-+--------------------+------------+-------------------+---------------+--------+
-
-* \(1) Outputs the number of the day of the week. Week begins on Monday and is 
denoted
-  by 0 and ends on Sunday denoted by 6.
++--------------------+------------+-------------------+---------------+--------+----------------------------+
+| Function name      | Arity      | Input types       | Output type   | Notes  
| Options class              |
++====================+============+===================+===============+========+============================+
+| year               | Unary      | Temporal          | Int64         |        
|                            |
++--------------------+------------+-------------------+---------------+--------+----------------------------+
+| month              | Unary      | Temporal          | Int64         |        
|                            |
++--------------------+------------+-------------------+---------------+--------+----------------------------+
+| day                | Unary      | Temporal          | Int64         |        
|                            |
++--------------------+------------+-------------------+---------------+--------+----------------------------+
+| day_of_week        | Unary      | Temporal          | Int64         | \(1)   
| :struct:`DayOfWeekOptions` |

Review comment:
       Tiny remark, but can you switch the order of the Notes and Option class 
columns (so the Notes is the last column). We just had a PR that made this 
consistent throughout the full document (#10630), so good to keep it consistent 
now.




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