stephstammel / consultthat

Automated tools to set up a new consulting project in R. Building on usethis: consultthat.
Other
18 stars 4 forks source link

Datetime formatting issue #2

Closed rosseji closed 6 years ago

rosseji commented 6 years ago

I found an issue that the punch on and off times didn't hold a consistent format.

Example

I made some changes to coerce the functions to store a numeric version of the system datetime and then for the timesheet() fun to report with the datetime format.

Example

Have made a pull request with some changes that could help.

rmflight commented 6 years ago

hey @rosseji, why the need for as.numeric (see here in punchOn), which you then convert back using as.POSIXct in the time reporting, when calling as.POSIXct on the time-string representation works just fine?

I don't know if it's a good idea, because it would make it almost impossible for timesheets to be modified or corrected by hand if the numeric representation is used.

rmflight commented 6 years ago
currtime <- 1526319604

tmp_time <- as.POSIXct(currtime, origin = "1970-01-01")
tmp_time
#> [1] "2018-05-14 13:40:04 EDT"
class(tmp_time)
#> [1] "POSIXct" "POSIXt"

tmp_time_2 <- as.POSIXct("2018-05-14 13:40:04 EDT")
tmp_time_2
#> [1] "2018-05-14 13:40:04 EDT"
class(tmp_time_2)
#> [1] "POSIXct" "POSIXt"
rmflight commented 6 years ago

whoops, ok, just looked at your examples, see why you did it.

Will see if I can come up with something that is consistent w/out using the numeric ....

rmflight commented 6 years ago

OK @rosseji and @stephdesilva , does this seem OK to you both:

library(consultthat)

tmp_loc <- tempfile()
dir.create(file.path(tmp_loc, "timetesting"), recursive = TRUE)
createProject(file.path(tmp_loc, "timetesting"), project_documents = "time", use_package = FALSE)

punchOn("Robert", project = file.path(tmp_loc, "timetesting"))
Sys.sleep(2)
punchOff("Robert", project = file.path(tmp_loc, "timetesting"))

punchOn("Robert", project = file.path(tmp_loc, "timetesting"))
Sys.sleep(1)
punchOff("Robert", project = file.path(tmp_loc, "timetesting"))

scan(file.path(tmp_loc, "timetesting", "project_documents", "time_management", "Robert_time_sheet.csv"), what = "character", sep = "\n")
#> [1] "\"project\",\"category\",\"notes\",\"punch_on\",\"punch_off\",\"state\""      
#> [2] "\"timetesting\",NA,NA,\"2018-05-14 15:16:41\",\"2018-05-14 15:16:43\",\"off\""
#> [3] "\"timetesting\",NA,NA,\"2018-05-14 15:16:43\",\"2018-05-14 15:16:44\",\"off\""

tmp <- timesheet("Robert", file.path(tmp_loc, "timetesting"))
tmp
#>       project category notes            punch_on           punch_off state
#> 1 timetesting       NA    NA 2018-05-14 15:16:41 2018-05-14 15:16:43   off
#> 2 timetesting       NA    NA 2018-05-14 15:16:43 2018-05-14 15:16:44   off

as.POSIXct(tmp$punch_on)
#> [1] "2018-05-14 15:16:41 EDT" "2018-05-14 15:16:43 EDT"
difftime(as.POSIXct(tmp$punch_off), as.POSIXct(tmp$punch_on))
#> Time differences in secs
#> [1] 2 1

This comes from this branch.

It looks like as.POSIXct will handle those date-strings, so we will be able to use them for actually doing difftime between punch_on and punch_off to get full sums of time spent on a project by person, which I know is a goal in the end, while being human readable and editable, which is a concern I have with time-keeping.

stephstammel commented 6 years ago

That makes sense to me - an end goal where the user can easily obtain human readable data on time spent would be beneficial. A lot of the users I had in mind (at the beginning at least) are likely to be newer to R/consulting so the simpler the better from their point of view, I suspect.

rmflight commented 6 years ago

Ok, I will merge the time stuff into the dot-consulthat branch and do it all as a single pull request.