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

Time change seems to be causing issues #130

Open
nrlottig opened this issue Jun 2, 2018 · 7 comments
Open

Time change seems to be causing issues #130

nrlottig opened this issue Jun 2, 2018 · 7 comments

Comments

@nrlottig
Copy link

nrlottig commented Jun 2, 2018

I'm getting the following error running k.read.base:

Error in data.frame(dateTime = dat$dateTime, C_D = C_D, alh = alh, ash = ash) :
arguments imply differing number of rows: 16035, 16036

I believe that I have tracked it down to calc.zeng line 20. My dataframe includes the following data

2017-03-12 02:00:00 which returns an NA for line 20 which then shortens the dataframe to 16035 rows. There also seem to be a whole bunch of errors that cascade off of that length mismatch (e.g., line 53 atm.press is 16036 while e_a(line 52) is 16035.

@nrlottig
Copy link
Author

nrlottig commented Jun 2, 2018

I just deleted that single row from my dataset and everything ran fine.

@jzwart
Copy link
Member

jzwart commented Jun 5, 2018

Hey Noah,
I think this is a daylight savings issue. 2017-03-12 from 02:00:00 to 02:59:59 didn't 'exist' when moving from CST to CDT. calc.zeng uses as.POSIXct() with your system's timezone to ensure that it is in POSIX format, which it doesn't know what to do with 2017-03-12 02:00:00 in American/Chicago timezone since that didn't exist.

I think an improvement would be to remove as.POSIXct() from calc.zeng and replace it with an error that says that datetime must be in POSIXct format before running code. Similar to addNAs function:

# =============================================================
if(any(dateL)){ # look for the date column
names(x)[dateL] <- "datetime"
}else{
# warning("No 'date' column found")
stop("No 'datetime' column found")
}
if(!"POSIXct"%in%class(x[,"datetime"])){ # make sure the date column is POSIXct (note that if date column is not found, you get this error)
stop("date column must be POSIXct")
}

@jzwart
Copy link
Member

jzwart commented Jun 5, 2018

what do others think? @lawinslow @rBatt @riwoolway

@rBatt
Copy link
Contributor

rBatt commented Jun 6, 2018 via email

@boegmanl
Copy link

boegmanl commented Jun 6, 2018 via email

@jzwart
Copy link
Member

jzwart commented Jun 6, 2018

If the user wants to put everything into GMT then that is an option for avoiding daylight savings issues, but I don't think the functions should have a timezone set to GMT. I think the user should get out the time zone that they put into the function.

@rBatt
Copy link
Contributor

rBatt commented Jun 18, 2018

The trick is getting things from some unknown or arbitrary time zone into something like GMT. Once in GMT, everything is easy, and I agree it's a simpler standard to work with. But I also agree that maybe we don't want to take on the task of converting everything to GMT ourselves.

Quick Aside: I worry about what time zone our sun.rise.set uses, though looking through the code again I think it's all standardized to local hours of day. So maybe not an issue.

Interestingly, in R's as.POSIXct, if you just specify the time zone code for the location, it'll automatically specify whether it's standard time or daylight time.

For example:

as.POSIXct("02/09/2018", format="%m/%d/%Y", tz="America/New_York")

"2018-02-09 EST"

as.POSIXct("06/09/2018", format="%m/%d/%Y", tz="America/New_York")

"2018-06-09 EDT"

Once a user has their data in POSIX format with the time zone specified, they might have an easier time solving the problem.

Maybe the solution here is to leave our code untouched, but start compiling a list of examples or FAQ's in the GitHub wiki that we can reference to show people how to make such a conversion in their own code when using LakeMetabolizer. I.e., just give an example with this initial problem, and show how to solve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants