Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Better Support for POSIXlt #3735

Closed
billdenney opened this issue Aug 2, 2018 · 7 comments
Closed

Feature Request: Better Support for POSIXlt #3735

billdenney opened this issue Aug 2, 2018 · 7 comments

Comments

@billdenney
Copy link
Contributor

I recently found what I thought was a bug but is apparently a feature where mutate (and perhaps more dplyr functions) do not support POSIXlt in columns. I think that I understand why due to POSIXlt being a list of vectors rather than a class more like a vector (while reading through prior issues here, that seems to be the case).

library(dplyr)                              
#> Warning: package 'dplyr' was built under R version 3.4.4
#> 
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:stats':
#> 
#>     filter, lag
#> The following objects are masked from 'package:base':
#> 
#>     intersect, setdiff, setequal, union
my_data <- data.frame(A=1, B=c(40000.5, NA))
my_data %>%                                 
mutate(B=as.POSIXlt(B, origin="2018-01-01"))
#> Warning: package 'bindrcpp' was built under R version 3.4.4
#> Error in mutate_impl(.data, dots): Column `B` is of unsupported class POSIXlt
my_data %>%                                 
mutate(B=as.POSIXct(B, origin="2018-01-01"))
#>   A                   B
#> 1 1 2018-01-01 06:06:40
#> 2 1                <NA>

Since POXIXlt is not supported, but POSIXct is, could POSIXlt columns be automatically converted to POSIXct with a warning rather than the less-usable error of "Column B is of unsupported class POSIXlt" (which doesn't give any hints about why it is unsupported or what to do to make a column supported).

@krlmlr

This comment has been minimized.

@billdenney

This comment has been minimized.

@krlmlr

This comment has been minimized.

@billdenney
Copy link
Contributor Author

It makes sense that auto-conversion in those places would be brittle changes.

Could the errors be made more directive? Rather than just saying “POSIXlt not supported” (and similar), could thy say “POSIXlt not supported, try converting to POSIXct.”? That could give the user a hint of how to solve the problem.

@hadley
Copy link
Member

hadley commented Aug 3, 2018

I agree that all we need here is an improved error message:

#> Error: Column `B` is unsupported class POSIXlt; use POSIXct instead.

Nothing in the tidyverse should ever produce a POSIXlt so this is most helpful for people with mixed base/tidyverse code.

@romainfrancois

This comment has been minimized.

@lock
Copy link

lock bot commented Nov 24, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Nov 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants