6
\$\begingroup\$

I've been working on a code that reads in all the sheets of an Excel workbook, where the first two columns in each sheet are "Date" and "Time", and the next two columns are either "Level" and "Temperature, or "LEVEL" and "TEMPERATURE". The code works, but I am working on improving my coding clarity and efficiency, so any advice in those regards would be greatly appreciated.

My function

  1. Reads in the data to a list of dataframes
  2. Gets rid of any NA columns that were accidentally read in.
  3. Combines "Date" and "Time" to "DateTime" for each dataframe
  4. Rounds "DateTime" to the nearest 5 minutes for each dataframe
  5. Replaces "Date" and "Time" in each dataframe with "DateTime".

I started getting more comfortable with lapply, but am wondering if I can improve the code efficiency at all instead of have so many lines with lapply.

library(readxl)
library(plyr)

  read_excel_allsheets <- function(filename) {
  sheets <- readxl::excel_sheets(filename)
  data <- lapply(sheets, function(X) readxl::read_excel(filename, sheet = X))
  names(data) <- sheets
  clean <- lapply(data, function(y) y[, colSums(is.na(y)) == 0])
  date <- lapply(clean, "[[", 1)
  time <- lapply(clean, "[[", 2)
  time <- lapply(time, function(z) format(z, format = "%H:%M"))
  datetime <- Map(paste, date, time)
  datetime <- lapply(datetime, function(a) as.POSIXct(a, format = "%Y-%m-%d %H:%M"))
  rounded <- lapply(datetime, function(b) as.POSIXlt(round(as.numeric(b)/(5*60))*(5*60),origin='1970-01-01'))
  addDateTime <- mapply(cbind, clean, "DateTime" = rounded, SIMPLIFY = F)
  final <- lapply(addDateTime, function(z) z[!(names(z) %in% c("Date", "Time"))])
  return(final)
}

Next, I would like to plot all of my data. So, I

  1. Tun my code for a file
  2. Combine the list of dataframes into one dataframe while maintaining an "ID" for each dataframe as a column
  3. Combine the lowercase and uppercase versions of the variable columns.
  4. Add two new columns that split the "ID". Each ID is something like B1CC or B2CO, where I want to split the "ID" like so: "B1" and "CC".

Now I can use ggplot very easily.

mysheets <- read_excel_allsheets(filename)
df = ldply(mysheets)
df$Temp <- rowSums(df[, c("Temperature", "TEMPERATURE")], na.rm = T)
df$Lev <- rowSums(df[, c("Level", "LEVEL")], na.rm = T)
df <- df[!names(df) %in% c("Level", "LEVEL", "Temperature", "TEMPERATURE")]

df$exp <- gsub("^[[:alnum:]]{2}", "\\1",df$.id)
df$plot <- gsub("[[:alnum:]]{2}$", "\\1", df$.id)

Here are the data for the first two dataframes, but there are over 50 of them, and each is relatively big, and there are many files to read. Therefore, I'm looking to improve efficiency (in terms of time to run) where I can.

dput(head(x[[1]]))
structure(list(Date = structure(c(1305504000, 1305504000, 1305504000, 
1305504000, 1305504000, 1305504000), class = c("POSIXct", "POSIXt"
), tzone = "UTC"), Time = structure(c(-2209121912, -2209121612, 
-2209121312, -2209121012, -2209120712, -2209120412), class = c("POSIXct", 
"POSIXt"), tzone = "UTC"), Level = c(106.9038, 106.9059, 106.89, 
106.9121, 106.8522, 106.8813), Temperature = c(6.176, 6.173, 
6.172, 6.168, 6.166, 6.165)), .Names = c("Date", "Time", "Level", 
"Temperature"), row.names = c(NA, 6L), class = c("tbl_df", "tbl", 
"data.frame"))

dput(head(x[[2]]))
structure(list(Date = structure(c(1305504000, 1305504000, 1305504000, 
1305504000, 1305504000, 1305504000), class = c("POSIXct", "POSIXt"
), tzone = "UTC"), Time = structure(c(-2209121988, -2209121688, 
-2209121388, -2209121088, -2209120788, -2209120488), class = c("POSIXct", 
"POSIXt"), tzone = "UTC"), LEVEL = c(117.5149, 117.511, 117.5031, 
117.5272, 117.4523, 117.4524), TEMPERATURE = c(5.661, 5.651, 
5.645, 5.644, 5.644, 5.645), `NA` = c(NA_real_, NA_real_, NA_real_, 
NA_real_, NA_real_, NA_real_), `NA` = c(NA_real_, NA_real_, NA_real_, 
NA_real_, NA_real_, NA_real_), `NA` = c(NA_real_, NA_real_, NA_real_, 
NA_real_, NA_real_, NA_real_), `NA` = c(NA_real_, NA_real_, NA_real_, 
NA_real_, NA_real_, NA_real_), `NA` = c(NA_real_, NA_real_, NA_real_, 
NA_real_, NA_real_, NA_real_)), .Names = c("Date", "Time", "LEVEL", 
"TEMPERATURE", NA, NA, NA, NA, NA), row.names = c(NA, 6L), class =    
c("tbl_df", "tbl", "data.frame"))
\$\endgroup\$
4
\$\begingroup\$

There are a few cons with writing code that uses lapply/Map/mapply at each step. You probably know that these functions are disguised for loops so it is as-if you where running for (i in 1:n) {...} at each line of your code. This will likely be:

  • slower than a single loop (and for loops are reputably slow in R...)
  • memory inefficient: in the event that each iteration creates a lot of data, that data will be stored in memory for all spreadsheets at the same time
  • more cumbersome to test and debug
  • more typing

Instead, I would suggest you write a function to process a single sheet and only loop once using that function (disclaimer: code could not be tested!):

library(readxl)

read_excel_allsheets <- function(filename) {

  process_sheet <- function(sheet, filename) {
    data <- readxl::read_excel(filename, sheet = sheet)
    data <- data[, colSums(is.na(data)) == 0])
    names(data) <- tolower(data)
    stopifnot(c("date", "time", "level", "temperature") %in% names(data))

    datetime <- with(data,
      as.POSIXct(paste(date, time), format = "%Y-%m-%d %H:%M"))
    rounded <- as.POSIXlt(round(as.numeric(datetime) / (5 * 60)) * (5 * 60),
                          origin = '1970-01-01'))

    cbind(datetime = rounded, data[c("level", "temperature")])
  }

  sheets <- readxl::excel_sheets(filename)
  Map(process_sheet, sheets, filename)
}

mysheets <- read_excel_allsheets(filename)
df <- do.call(rbind, Map(cbind, id = names(mysheets), mysheets))
df$plot <- substr(df$id, start = 1, stop = 2)
df$exp  <- substr(df$id, start = 3, stop = 4)

Here are other improvements I made:

  1. Rather than carry the weird assumptions about your data column names into the second part of your code, I took care of making uniform names in the data reading/cleaning section. For that, I converted all names to lowercase using tolower.
  2. I added a stopifnot() to check that each sheet contains the four columns you need. Always try to gather in your head the assumptions that your code is relying on, then add code to check these assumptions and die if they are not met.
  3. Everywhere I needed a column from the data, I referred to it using its name (e.g. "date") rather than its position (e.g. 1). Doing so will always make your code more readable and more robust to data changes.
  4. I replaced plyr::ldply with a base solution. It helps remove a dependency to a 3rd party package but if you prefer to keep dplyr, it's ok too.
  5. I replaced the gsub calls with simpler (easier to read) substr calls. I note that your gsub calls were probably returning the expected results but you are doing something weird. To use \\1, your regex pattern should really use a set of parenthesis () to catch something that will be stored inside \\1. It would have made more sense to not use regex captures and just use the empty string "" as the replacement instead of "\\1".
\$\endgroup\$
  • \$\begingroup\$ thank you so much, I have learned a lot from your response. Very helpful! \$\endgroup\$ – user2943039 Nov 1 '15 at 2:41
  • \$\begingroup\$ Just FYI, I system.time'd the difference between your code suggestions and mine. Yours was 46.65, mine was 48.51...so actually not a huge difference, but I definitely learned from your post. Thank you! \$\endgroup\$ – user2943039 Nov 1 '15 at 3:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.