stplanr vulnerable to forthcoming changes in sp and rgdal

[rsbivand]

Running revdep checks for current rgdal on R-Forge - see:

https://stat.ethz.ch/pipermail/r-sig-geo/2019-November/027801.html

shows the errors in the attached check log, related to use of PROJ&/GDAL3
and required changes to sp and rgdal. If useful find a regerence to a docker
image in this thread:

r-spatial/discuss#28

Changes will occur quite fast, and packages need to be prepared.

* checking examples ... ERROR
Running examples in ‘stplanr-Ex.R’ failed
The error most likely occurred in:

> ### Name: buff_geo
> ### Title: Create a buffer of n metres for non-projected 'geographical'
> ###   spatial data
> ### Aliases: buff_geo
> 
> ### ** Examples
> 
> r <- routes_fast[1:3, ]
> buff <- buff_geo(r, width = 100)
Transforming to CRS +proj=aeqd +lat_0=53.82763236 +lon_0=-1.53163002 +x_0=0 +y_0=0 +datum=WGS84 +units=m +no_defs
Warning in spTransform(xSP, CRSobj, ...) :
  NULL source CRS comment, falling back to PROJ string
proj_create: init=epsg:/init=IGNF: syntax not supported in non-PROJ4 emulation mode
Error in .spTransform_Line(input[[i]], to_args = to_args, from_args = from_args,  : 
  source crs creation failed: (null)
Calls: buff_geo ... spTransform -> .spTransform_Lines -> .spTransform_Line
Execution halted

[Robinlovelace]

Thanks for the heads-up Roger, I'll brace myself and test things on the Docker container asap.

[Robinlovelace]

Update on this: I get no errors when testing in @Nowosad's docker container that I started with:

docker run -d -p 8789:8787 -e DISABLE_AUTH=TRUE -v $(pwd):/home/rstudio/  jakubnowosad/geocompr_proj6

Not tried on latest version of rgdal though, that's the next step I guess...

[Robinlovelace]

Just installed latest versions with:

devtools::install_github("rsbivand/sp")
install.packages("rgdal", repos="http://R-Forge.R-project.org")

[Robinlovelace]

Confirmed: I can reproduce the error.

[Robinlovelace]

Heads-up @rsbivand, this is the offending PROJ string:

CRS arguments:
 +proj=aeqd +lat_0=53.81895189 +lon_0=-1.52748741 +x_0=0 +y_0=0 +datum=WGS84 +units=m +no_defs 

Reproducible example:

library(stplanr)
data("routes_fast")
new_crs <- geo_select_aeq(routes_fast)
new_crs2 <- geo_select_aeq(routes_fast) # to be deprecated
identical(new_crs, new_crs2)
#> [1] TRUE
new_crs
#> CRS arguments:
#>  +proj=aeqd +lat_0=53.81895189 +lon_0=-1.52748741 +x_0=0 +y_0=0
#> +datum=WGS84 +units=m +no_defs
plot(routes_fast)

rf_projected <- sp::spTransform(routes_fast, new_crs)
#> Warning in spTransform(xSP, CRSobj, ...): NULL source CRS comment, falling
#> back to PROJ string
#> Error in .spTransform_Line(input[[i]], to_args = to_args, from_args = from_args, : source crs creation failed: Unknown error 1152454384

^{Created on 2019-11-16 by the reprex package (v0.3.0)}

[rsbivand]

Good, thanks for the updates. It is not impossible that the changes in **sp** and **rgdal** should be wrapped up to moderate the collateral. Roger Roger Bivand Falsensvei 32 5063 Bergen lør. 16. nov. 2019, 23.51 skrev Robin <notifications@github.com>:

…

Confirmed: I can reproduce the error. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#364?email_source=notifications&email_token=ACNZ3BBD3KQDZJMUB5YTBCDQUB2NNA5CNFSM4JOGOH6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEH4KXI#issuecomment-554681693>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACNZ3BBB5PM45KVIJ5EVRMDQUB2NNANCNFSM4JOGOH6A> .

[Robinlovelace]

No problem. More findings that may be of use/interest below: the function fails with spTransform() but not st_transform() seemingly:

library(stplanr)
buff_sp <- geo_buffer(routes_fast, width = 100)
#> Transforming to CRS +proj=aeqd +lat_0=53.81895189 +lon_0=-1.52748741 +x_0=0 +y_0=0 +datum=WGS84 +units=m +no_defs
#> Warning in spTransform(xSP, CRSobj, ...): NULL source CRS comment, falling
#> back to PROJ string
#> Error in .spTransform_Line(input[[i]], to_args = to_args, from_args = from_args, : source crs creation failed: invalid projection system error (-9999)
routes_fast_sf <- sf::st_as_sf(routes_fast)
#> Warning in CPL_crs_from_proj4string(x): GDAL Message 1: +init=epsg:XXXX
#> syntax is deprecated. It might return a CRS with a non-EPSG compliant axis
#> order.
buff_sf <- geo_buffer(routes_fast_sf, dist = 50)
plot(buff_sf$geometry, add = TRUE)
#> Error in polypath(p_bind(x[[i]]), border = border[i], lty = lty[i], lwd = lwd[i], : plot.new has not been called yet

^{Created on 2019-11-16 by the reprex package (v0.3.0)}

[Robinlovelace]

FYI @rsbivand the functions generating those (now faulty?) PROJ strings are pasted below. I'm not sure what the next step is now we've roughly identified the culprit of the crashes, which is just the first step I guess.

#' @export
geo_select_aeq <- function(shp) {
  UseMethod("geo_select_aeq")
}
#' @export
geo_select_aeq.Spatial <- function(shp) {
  cent <- rgeos::gCentroid(shp)
  aeqd <- sprintf(
    "+proj=aeqd +lat_0=%s +lon_0=%s +x_0=0 +y_0=0",
    cent@coords[[2]], cent@coords[[1]]
  )
  sp::CRS(aeqd)
}
#' @export
geo_select_aeq.sf <- function(shp) {
  cent <- sf::st_geometry(shp)
  coords <- sf::st_coordinates(shp)
  coords_mat <- matrix(coords[, 1:2], ncol = 2)
  midpoint <- apply(coords_mat, 2, mean)
  aeqd <- sprintf(
    "+proj=aeqd +lat_0=%s +lon_0=%s +x_0=0 +y_0=0",
    midpoint[2], midpoint[1]
  )
  sf::st_crs(aeqd)
}

[rsbivand]

Problems still present with 0.5-0. Testing with sf 0.8-1, my sp 1.3-4 and rgdal 1.5-3:

* checking examples ... ERROR
Running examples in ‘stplanr-Ex.R’ failed
The error most likely occurred in:

> ### Name: od_radiation
> ### Title: Function that estimates flow between points or zones using the
> ###   radiation model
> ### Aliases: od_radiation
> 
> ### ** Examples
> 
> # load some points data
> data(cents)
> # plot the points to check they make sense
> plot(cents)
> class(cents)
[1] "SpatialPointsDataFrame"
attr(,"package")
[1] "sp"
> # Create test population to model flows
> set.seed(2050)
> cents$population <- runif(n = nrow(cents), min = 100, max = 1000)
> # estimate
> flowlines_radiation <- od_radiation(cents, pop_var = "population")
Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO") :
  Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved
Transforming to CRS +proj=aeqd +lat_0=53.81056376 +lon_0=-1.52916207 +x_0=0 +y_0=0 +datum=WGS84 +units=m +no_defs
Transforming to CRS +proj=aeqd +lat_0=53.80951653 +lon_0=-1.54646278 +x_0=0 +y_0=0 +datum=WGS84 +units=m +no_defs
Warning in spTransform(xSP, CRSobj, ...) :
  NULL source CRS comment, falling back to PROJ string
Warning in spTransform(xSP, CRSobj, ...) : +init dropped in PROJ string
Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO") :
  Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved
Error in over(x, geometry(i)) : identicalCRS(x, y) is not TRUE
Calls: od_radiation -> [ -> [ -> over -> over -> stopifnot
Execution halted

[Robinlovelace]

flowlines_radiation <- od_radiation(cents, pop_var = "population")

That crashing code uses sp. I suggest simply not running the example and deprecating it. Way more efficient implementations are possible. Sound like a plan? Many thanks for testing @rsbivand

[Robinlovelace]

Update on this: I plan to make further changes, including updating the remain sp objects in stplanr when I get the latest version of rgdal installed.

[Robinlovelace]

Closing for now, not because I think the issues are fixed but because I think deprecating the the failing sp code and gradually dropping support for sp seems to be working as a strategy. Very few people using sp any more and nobody I think using it for mission critical work. If any other issues re-appear I'm happy to look at this again but I think long-term my strategy is to remove the dependency on sp altogether as per #332.

[Robinlovelace]

Still an issue:

Dear maintainer,

Your package stplanr stores data objects using classes defined
in the sp package or inheriting from those classes. From sp 1.4-* and
rgdal 1.5-*, CRS objects are now expected to have a comment containing
an WKT2 CRS representation - see:

https://www.r-spatial.org/r/2020/03/17/wkt.html

A section at the foot of

https://rgdal.r-forge.r-project.org/articles/CRS_projections_transformations.html#rebuilding-crs-objects

describes possible steps for updating CRS objects; earlier sections cover
background issues. Working around sp objects with stale CRS is already
causing difficulties, so your cooperation in upgrading will be helpful.An attempt has been made to rebuild data objects using sp::rebuild_CRS(),
and output (re-stored as version 2) may be found on my server at:

http://spatial.nhh.no/R/rebuilt_sp_objects/

The functions used are from the development version of rgdal, 1.5-17,
rev >= 1053, use:

install.packages("rgdal", repos="http://R-Forge.R-project.org")

and:

remotes::install_github("rsbivand/sp")

should you wish to make changes. Please check carefully that these objects
are as you would expect. Note that the rebuilt CRS object contains
a revised version of the input Proj4 string as well as the WKT2 string,
and may be used with both older and newer versions of sp. Objects
inheriting from sp classes but with NA CRS are not modified.

[Robinlovelace]

New warning message from new CRSs with devtools::check():

* checking data for ASCII and uncompressed saves ... WARNING
  Warning: large data files saved inefficiently:
             size ASCII compress
  cents.rda 160Kb FALSE     none
  zones.rda 334Kb FALSE     none

Heads-up @rsbivand (many thanks for excellent guidance on updating stale CRSs).

[Robinlovelace]

Good news, this seems to have fixed it:

devtools::use_data(cents)
devtools::use_data(zones)

Should be ready to merge the PR soon: #421

[rsbivand]

The pre-cooked *.rda were saved in version 2. Version 3 forces a dependency on R >= 3.5.0, which I wished to avoid. Like many of sps rev-deps, stplanr is at R >= 3.0. I think that devtools is simply wrong to require recompression (probably to version 3). Running R CMD check --as-cran on spgwr after adding in my rebuilt sp objects with released R shows no such warning, nor does checking as CRAN incoming with R devel from this week. I've never trusted devtools and still do not. I'd suggest checking the save version on your fix; if you want to keep R >= 3.0.2, you'd need to go back to version 2.

[Nowosad]

@Robinlovelace I think, you can just use save with some additional arguments to (1) keep version 2, and (2) compress the output file. For example:

save(cents, file = "data/cents.rds", version = 2, compress = "bzip2")

[Robinlovelace]

Thanks for the explanation, plan to set R >= 3.5.0 as a dependency.

#422

I think the warning happened on large (> 100 kB?) objects and am confident it would cause the CRAN checks to fail.

Just saw Jakub's email. That would allow both compression and an older version. Given that sf depends on R > 3.3.0 will it make that much difference to users? If so I'll go with your solution, thanks for quickfire input Jakub!

[rsbivand]

Just submitted spgwr with a 300K sp object - let's see ...