zoonproject / zoon

The zoon R package
Other
61 stars 13 forks source link

InteractiveCovariateMap refers to df$lon and df$lat which R attempts to autocomplete #380

Open timcdlucas opened 7 years ago

timcdlucas commented 7 years ago

This breaks when columns are added with names that make df$lon ambiguous.

My current test module for example


LonLatToCovariates <- function (.data, which_covs = "pairs", exclude = NULL) {

  df <- .data$df
  ras <- .data$ras

  # create copy of rasters
  lonlat <- raster::stack(ras[[1]], ras[[1]])

  # Add new values to layers
  values(lonlat) <- raster::rasterToPoints(lonlat)[, 1:2]

  # rename layerse
  names(lonlat) <- c('longitudecov', 'latitudecov')

  # Stack
  ras <- raster::stack(ras, lonlat)

  vals <- raster::extract(lonlat, df[, c("longitude", "latitude")])
  df <- zoon:::cbindZoon(df, vals)
  attr(df, 'covCols') <- c(attr(df, 'covCols'), c('longitudecov', 'latitudecov'))

  return(list(df = df, ras = ras))
}

work1 <- workflow(occurrence = UKAnophelesPlumbeus,
                  covariate = UKAir,
                  process = Background(n = 70),
                  model = LogisticRegression,
                  output = list(SameTimePlaceMap,
                                InteractiveCovariateMap))

work2 <- workflow(occurrence = UKAnophelesPlumbeus,
                  covariate = UKAir,
                  process = Chain(Background(n = 70),
                                  LonLatToCovariates),      
                  model = LogisticRegression,
                  output = list(SameTimePlaceMap,
                                InteractiveCovariateMap))

This breaks because my column names longitudecov and latitudecov conflict.

Not sure if other modules have this same issue. For now I'll just capitlise them I think... might try to go back and fix modules sometime.

goldingn commented 7 years ago

Bugger. Doing this in InteractiveMap and other output modules should do it:

df[, c('lon', 'lat')]

If we're going to rely on module writers to do this, we could provide accessor functions and suggest they be used. Similarly, worth adding one to get the covariate values using the covCols attribute since that's used a lot and is non-obvious.

Also worth adding a lat/lon covariates module in the module tests used by BuildModule, to catch this early.

timcdlucas commented 7 years ago

Yup agree on all. Though it'd be

df[, c('longitude', 'latitude')]

Possibly a more general test could be a workflow that creates random covariates with awkward names.