jorisvandenbossche commented on pull request #10176:
URL: https://github.com/apache/arrow/pull/10176#issuecomment-854529392


   To recap some spread inline comments (original discussion at 
https://github.com/apache/arrow/pull/10176#discussion_r633497393) about the 
sub-second / fractional components. 
   
   Assume the timestamp `2012-03-26 01:02:03.123456789`.
   
   Currently, I think the PR does:
   
   * second -> 3.123456789 (float)
   * subsecond -> 123456789 (int)
   
   Based on a naive reading of the docstrings and my biased expectations, I 
would have thought they return:
   
   * second -> 3 (int)
   * subsecond -> 0.123456789 (float)
   
   
   Now, in our previous discussion you proposed to implement `subsecond` as 
"total units since last second", so that indeed matches with what is in the PR 
now. But, that also means that the interpretation of this value critically 
depends on the unit of the timestamp type. While a float value would not have 
that problem.
   
   In addition, I personally think it would be useful to have some form of 
`second` that is just the integer the non-fractional component in " 
01:02:03.123...".
   
   ---
   
   In addition, we also have the other components (for which eg Antoine 
questioned whether they are needed):
   
   * microsecond -> 123
   * millisecond -> 456
   * nanosecond -> 789


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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to