filter_ems_data date conversion

[sebdalgarno]

these lines in filter_ems_data:

  if (!is.null(from_date)) from_date <- as.POSIXct(from_date, tz = ems_tz())
  if (!is.null(to_date)) to_date <- as.POSIXct(to_date, ems_tz())

causes the date to be changed.

e.g.

from_date = as.Date("2019-11-18")
as.POSIXct(from_date, tz = rems:::ems_tz())

# returns
"2019-11-17 16:00:00 PST"

Is this the desired behaviour?

This is causing issues in the ShinyRems app, where if the user searches for data on 2019-11-18 (and data does exist that day) no data is returned.

[sebdalgarno]

COLLECTION_START is POSIXct with 9:00:00 arbitrarily assigned to date. Should user-provided dates be treated like this instead?

[ateucher]

it's the as.POSIXct.Date() S3 method that's doing strange things, so there is a case there to catch. The documentation for the function though says:

from_date    A date string in a standard unambiguous format (e.g., "YYYY/MM/DD")

to_date      A date string in a standard unambiguous format (e.g., "YYYY/MM/DD")

And

from_date = "2019-11-18"
as.POSIXct(from_date, tz = rems:::ems_tz())

# returns
[1] "2019-11-18 -08"

[ateucher]

COLLECTION_START is POSIXct with 9:00:00 arbitrarily assigned to date

I'm not sure what you mean? COLLECTION_START can contain any valid date/time

[sebdalgarno]

re: COLLECTION_START you are totally correct! ignore what I said.

re: as.POSIXct.Date() Thanks for looking into it. For the time being I will pass a character string instead of class Date object. But yes, I wonder if the function should either convert Date objects to character or return error when dates not passed as character strings.

[sebdalgarno]

users (like me!) should read the documentation but is maybe a little worrying that it silently (sort of) works

[ateucher]

Yup, agreed it needs to be fixed!

[sebdalgarno]

I don't mind making a pull request if you let me know how you want to approach it

[sebdalgarno]

we could introduce a dependency on chk package and use it to check function args - e.g. chk::chk_string(from_date) eventually other functions could be updated to incorporate chk

[ateucher]

No, I'd like it to work with Date objects, I think it's worth it