Upcoming sf breaks fasterize

[rsbivand]

Upcoming sf changes the "crs" object which will not have a "proj4string" component. See: https://github.com/edzer/sp/issues/73 for example found running revdeps with sf 0.8-2 probably SetFromUserInput branch.

[mdsumner]

thanks @rsbivand

@noamross I suggest removing the two blocks that copy the sf crs to the raster, it's currently doing the wrong thing if the sf and raster objects do not share the same projection (assigning the polygon's projection to the raster, no checks are done), and so this is more consistent anyway. If users need to project the polygons to the grid they can do that upfront.

If you agree I can PR it - I'll look at adding some logic to compare the crs, fall back etc. - but the same issue will affect the raster package too - so might need to wait and see what happens there.

[noamross]

Thanks @rsbivand. @mdsumner that would be great. I've been meaning to get to this but have been traveling and will be on vacation for the next week. Will review and get it up as soon as I can when I return, along with another fix that is needed to comply with new CRAN compiler requirements.

[mdsumner]

Testing on sf@jeroen-gdal3-rwinlib I see

test-02-fasterize.R:41: failure: projections handled properly
st_crs(nc)$proj4string not equal to proj4string(r_nc).
1/1 mismatches
x[1]: "+proj=longlat +datum=NAD27 +no_defs"
y[1]: "+proj=longlat +datum=NAD27 +no_defs +ellps=clrk66 +nadg
y[1]: rids=@conus,@alaska,@ntv2_0.gsb,@ntv1_can.dat"
--------------------------------------------------------------
v |   2       | rastermethods
v |   8       | funs
x |  12 1     | group-by operations [0.3 s]
--------------------------------------------------------------
test-05-by.R:58: failure: projections handled properly with by
st_crs(nc)$proj4string not equal to proj4string(r_nc).
1/1 mismatches
x[1]: "+proj=longlat +datum=NAD27 +no_defs"
y[1]: "+proj=longlat +datum=NAD27 +no_defs +ellps=clrk66 +nadg
y[1]: rids=@conus,@alaska,@ntv2_0.gsb,@ntv1_can.dat"

Because now (PROJ6 in sp, and sf but still with current sf structure ) we see old behaviour in sf but new behaviour in sp:

f <- system.file("gpkg/nc.gpkg", package = "sf", mustWork = TRUE)
sf::st_crs(sf::read_sf(f))
Coordinate Reference System:
  EPSG: 4267 
  proj4string: "+proj=longlat +datum=NAD27 +no_defs"

## but
 sp::CRS(sp::proj4string(r_nc))
CRS arguments:
 +proj=longlat +datum=NAD27 +no_defs +ellps=clrk66
+nadgrids=@conus,@alaska,@ntv2_0.gsb,@ntv1_can.dat 

Suggest just remove these tests, since they break the new sf structure anyway (sorry I missed in the PR).

I also see

test-01-inputcheck.R:28: warning: field value name is in sf object
`fasterize(pols, r1, field = "hello")` generated an S3 error and you are testing the error message.
* The error has class = c("Rcpp::index_out_of_bounds", "C++Error", "error", "condition")
* Testing with `class` is more robust than testing with `regexp`.
* Do you want `expect_error(..., class = "Rcpp::index_out_of_bounds")`?

(Doing that makes that test pass).

[rsbivand]

Good. I can't see whether expect_error(..., class = *) can handle objects inheriting from more than one class. ?expect_error says: "A more robust way is to test for the class of the error, if it has one." I don't know whether this is using the logic of inherits() returning a logical or class() returning a vector whose length may be > 1.

[mdsumner]

I was wondering that myself, I'll be looking at this again on @noamross's behalf so I'll check this out properly.

[edzer]

I've submitted sf to CRAN; this causes an error in package NLMR, see https://win-builder.r-project.org/incoming_pretest/sf_0.9-0_20200319_213604/reverseDependencies/summary.txt

[edzer]

Pkg rasterDT also affected, see link above.

[edzer]

Also package reproducible, see link above.