Trackage / trip

trip package development
http://trackage.github.io/trip/
12 stars 2 forks source link

trip vulnerable to forthcoming changes in sp and rgdal #39

Closed rsbivand closed 4 years ago

rsbivand commented 4 years ago

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:

https://github.com/r-spatial/discuss/issues/28

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

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

> ### Name: rasterize
> ### Title: Rasterize trip objects based on line-segment attributes.
> ### Aliases: rasterize rasterize,trip,RasterLayer-method
> ###   rasterize,trip,missing-method
> 
> ### ** Examples
> 
>  d <- data.frame(x=1:10, y=rnorm(10), tms=Sys.time() + 1:10, id=gl(2, 5))
> sp::coordinates(d) <- ~x+y
> ## this avoids complaints later, but these are not real track data (!)
> sp::proj4string(d) <- sp::CRS("+proj=laea +ellps=sphere")
Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO") :
  Discarded datum Unknown_based_on_Normal_Sphere_r_6370997_ellipsoid in CRS definition
> tr <- trip(d, c("tms", "id"))
> 
> tr$temp <- sort(runif(nrow(tr)))
> r <- rasterize(tr)
Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO") :
  Discarded datum Unknown_based_on_Normal_Sphere_r_6370997_ellipsoid in CRS definition
> 
> rasterize(tr, grid = r)
Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO") :
  Discarded datum Unknown_based_on_Normal_Sphere_r_6370997_ellipsoid in CRS definition
class      : RasterLayer 
dimensions : 100, 100, 10000  (nrow, ncol, ncell)
resolution : 0.09090909, 0.02455464  (x, y)
extent     : 0.9545455, 10.04545, -0.8479059, 1.607558  (xmin, xmax, ymin, ymax)
crs        : +proj=laea +lat_0=0 +lon_0=0 +x_0=0 +y_0=0 +ellps=sphere +units=m +no_defs 
source     : memory
names      : z 
values     : 0, 0.09090909  (min, max)

> rasterize(tr, r, field = "temp")
Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO") :
  Discarded datum Unknown_based_on_Normal_Sphere_r_6370997_ellipsoid in CRS definition
Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO") :
  Discarded datum Unknown_based_on_Normal_Sphere_r_6370997_ellipsoid in CRS definition
Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO") :
  Discarded datum Unknown_based_on_Normal_Sphere_r_6370997_ellipsoid in CRS definition
Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO") :
  Discarded datum Unknown_based_on_Normal_Sphere_r_6370997_ellipsoid in CRS definition
Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO") :
  Discarded datum Unknown_based_on_Normal_Sphere_r_6370997_ellipsoid in CRS definition
Error in .linesToRaster(x, y, field = field, fun = fun, background = background,  : 
  lines and raster have no overlapping areas
Calls: rasterize ... .local -> rasterize -> rasterize -> .local -> .linesToRaster
Execution halted

There were test failures too:


R version 3.6.1 (2019-07-05) -- "Action of the Toes"
Copyright (C) 2019 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> library(testthat)
> library(trip)
> 
> test_check("trip")
── 1. Failure: speedfilter and sdafilter works (@test-algos.R#23)  ─────────────
`sda(tr, smax = 1000, pre = filt1)` produced warnings.

── 2. Failure: print and show (@test-methods.R#16)  ────────────────────────────
`{ ... }` produced warnings.

Longitudes contain values greater than 180, assuming proj.4 +over

 Data fully validated: returning object of class  trip 
── 3. Failure: trip works (@test-trip.R#40)  ───────────────────────────────────
`sp::spTransform(walrus818[1:100, ], "+proj=laea +datum=WGS84")` produced warnings.

── 4. Failure: plot works (@test-trip.R#52)  ───────────────────────────────────
`{ ... }` produced warnings.

══ testthat results  ═══════════════════════════════════════════════════════════
[ OK: 125 | SKIPPED: 0 | WARNINGS: 61 | FAILED: 4 ]
1. Failure: speedfilter and sdafilter works (@test-algos.R#23) 
2. Failure: print and show (@test-methods.R#16) 
3. Failure: trip works (@test-trip.R#40) 
4. Failure: plot works (@test-trip.R#52) 

Error: testthat unit tests failed
Execution halted
mdsumner commented 4 years ago

In summary I think required changes are

There doesn't appear to be a way to avoid this warning without having a +datum token? There's no sphere datum afaics.

The culprit in the package is walrus818,

walrus818@proj4string
CRS arguments:
 +proj=aeqd +ellps=WGS84 +lon_0=-170 +lat_0=70 
rsbivand commented 4 years ago

I'll get back later more fully. If there is a populated SRS, and exportToProj4() is called, it deletes +datum= silently in all cases other than WGS84, NAD83 and NAD27. This makes the PROJ string representation read from file inherently unsafe, especially as WGS84 is not clearly defined.

rsbivand commented 4 years ago

Very much later!

Still errors in the tests in your github/master with PROJ 7, GDAL 3, sp 1.4-1 (CRAN), rgdal 1.5-6 (R-forge), but no errors now in examples:

> library(testthat)
> library(trip)
> 
> test_check("trip")
── 1. Failure: speedfilter and sdafilter works (@test-algos.R#23)  ─────────────
`sda(tr, smax = 1000, pre = filt1)` produced warnings.

── 2. Failure: print and show (@test-methods.R#16)  ────────────────────────────
`{ ... }` produced warnings.

Longitudes contain values greater than 180, assuming proj.4 +over

 Data fully validated: returning object of class  trip 
── 3. Failure: trip works (@test-trip.R#40)  ───────────────────────────────────
`sp::spTransform(walrus818[1:100, ], "+proj=laea +datum=WGS84")` produced warnings.

── 4. Failure: plot works (@test-trip.R#52)  ───────────────────────────────────
`{ ... }` produced warnings.

══ testthat results  ═══════════════════════════════════════════════════════════
[ OK: 125 | SKIPPED: 0 | WARNINGS: 99 | FAILED: 4 ]
1. Failure: speedfilter and sdafilter works (@test-algos.R#23) 
2. Failure: print and show (@test-methods.R#16) 
3. Failure: trip works (@test-trip.R#40) 
4. Failure: plot works (@test-trip.R#52) 

Error: testthat unit tests failed
Execution halted
mdsumner commented 4 years ago

Now passing

mdsumner commented 4 years ago

Note to self follow up trail here: https://github.com/hypertidy/reproj/issues/8

and here: https://github.com/rspatial/raster/issues/87#issuecomment-638547200

mdsumner commented 4 years ago

Using this to monitor sneaking loading

sort(names(sessionInfo()$loadedOnly))
mdsumner commented 4 years ago

because it coerces to Lines:

as(walrus818, "SpatialLinesDataFrame")
class       : SpatialLinesDataFrame 
features    : 14 
extent      : -117277, 307789, -412557, 84896  (xmin, xmax, ymin, ymax)
crs         : +proj=aeqd +lat_0=70 +lon_0=-170 +x_0=0 +y_0=0 +datum=WGS84 +units=m +no_defs 
variables   : 4
names       : tripID,  tripStart,    tripEnd, tripDur 
min values  :    353, 1252987200, 1254337200, 1195200 
max values  :    444, 1283878800, 1287601200, 5695200 
Warning messages:
1: In proj4string(from) : CRS object has comment, which is lost in output
2: In proj4string(x) : CRS object has comment, which is lost in output
mdsumner commented 4 years ago

Removed a bunch of CRS() checks, "+over" and "+init" instances

mdsumner commented 4 years ago

fixed, thanks @rsbivand !

rsbivand commented 4 years ago

I expect to provide the same option before loading (think .Rprofile) to mute sp warnings like In proj4string(from) : CRS object has comment, which is lost in output as in rgdal - sp will act like rgdal if the option (mentioned in rgdal's startup messages) is found.