jonkeane commented on code in PR #13070:
URL: https://github.com/apache/arrow/pull/13070#discussion_r870332856


##########
r/tests/testthat/test-dplyr-funcs-datetime.R:
##########
@@ -1490,8 +1490,10 @@ test_that("`as.Date()` and `as_date()`", {
     dt_utc = ymd_hms("2010-08-03 00:50:50"),
     date_var = as.Date("2022-02-25"),
     difference_date = ymd_hms("2010-08-03 00:50:50", tz = "Pacific/Marquesas"),
-    character_ymd_var = "2022-02-25 00:00:01",
-    character_ydm_var = "2022/25/02 00:00:01",
+    character_ymd_hms_var = "2022-02-25 00:00:01",
+    character_ydm_hms_var = "2022/25/02 00:00:01",
+    character_ymd_var = "2022-02-25",
+    character_ydm_var = "2022/25/02",
     integer_var = 32L,

Review Comment:
   We should add at least one test with multiple dates (and especially one with 
multiple formats, like `c("2022-01-23", "2022/01/23")`) I suspect that will 
surface some interesting consequences for the operational parts of this code. 
For example:
   
   ```
   > as.Date(c("2022-01-01", "2022/01/01"), tryFormats = c("%Y-%m-%d", 
"%Y/%m/%d"))
   [1] "2022-01-01" NA     
   > lubridate::as_date(c("2022-01-01", "2022/01/01"))
   [1] "2022-01-01" "2022-01-01"
   ```
   
   We don't need to necessarily change this particular fixture (though having 
multiple rows is generally a good idea that we probably should have caught 
earlier...). But we should definitely have tests for this behavior.



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