gaogaotiantian commented on PR #59362:
URL: https://github.com/apache/spark/pull/59362#issuecomment-6511050589

   Thanks for adding these. Two suggestions:
   
   1. `assert_series_equal` doesn't check element types: a nested 
`datetime.datetime` and a `pd.Timestamp` for the same instant compare equal. 
But the main difference between the nested `TimestampType` and 
`TimestampLTZNanosType` from-pandas converters is the return type 
(`to_pydatetime()` vs keeping `pd.Timestamp`). Could you assert it explicitly? 
Note that `pd.Timestamp` subclasses `datetime`, so use `type(...) is`:
      ```python
      self.assertIs(type(result[0][0]), datetime.datetime)  # TimestampType
      self.assertIsInstance(result[0][0], pd.Timestamp)     # 
TimestampLTZNanosType
      ```
   
   2. In `test_converter_from_pandas_nested_timestamp`, the nanosecond check 
uses an already tz-aware input, which only exercises the branch that wraps the 
value in `pd.Timestamp`. The branch that could actually lose nanoseconds is the 
naive one (`tz_localize(timezone)`). Could you add a naive input such as 
`pd.Timestamp("2020-01-01 12:00:00.000000001")` and check that its nanoseconds 
survive?


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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to