search

ateucher / lutz

Look up timezones of point coordinates
http://andyteucher.ca/lutz/
Other
58 stars 5 forks source link

vulnerability to PROJ6/GDAL3 #7

Closed rsbivand closed 1 year ago

rsbivand commented 4 years ago

Running revdeps from sp (sp (my github fork) with development rgdal from R-Forge):

> library(testthat)
> library(lutz)
> 
> test_check("lutz")
Error : foo is not a valid time zone. See ?OlsonNames
OGR: Corrupt data
── 1. Error: tz_lookup.sf works (@test-tz_lookup2.R#36)  ───────────────────────
OGR error
Backtrace:
  1. testthat::expect_equal(...)
  4. lutz::tz_lookup(pt, 3005, method = "accurate")
  6. lutz:::tz_lookup_accurate.sfc(x, crs)
  8. lutz:::tz_lookup_accurate.sf(x_sf, crs = crs)
  9. lutz:::fix_sf(x, crs)
 10. sf:::Ops.crs(sf::st_crs(x), sf::st_crs(4326))
 11. sf:::Ops.crs(e1, e2)
 12. sf:::CPL_crs_equivalent(e1$proj4string, e2$proj4string)

OGR: Corrupt data
── 2. Error: tz_lookup.SpatialPoints works (@test-tz_lookup2.R#52)  ────────────
OGR error
Backtrace:
  1. testthat::expect_equal(...)
  4. lutz::tz_lookup(pt, 3005, method = "accurate")
  6. lutz:::tz_lookup_accurate.SpatialPoints(x, crs)
  8. lutz:::tz_lookup_accurate.sf(x, crs)
  9. lutz:::fix_sf(x, crs)
 10. sf:::Ops.crs(sf::st_crs(x), sf::st_crs(4326))
 11. sf:::Ops.crs(e1, e2)
 12. sf:::CPL_crs_equivalent(e1$proj4string, e2$proj4string)

══ testthat results  ═══════════════════════════════════════════════════════════
[ OK: 1084 | SKIPPED: 0 | WARNINGS: 12 | FAILED: 2 ]
1. Error: tz_lookup.sf works (@test-tz_lookup2.R#36) 
2. Error: tz_lookup.SpatialPoints works (@test-tz_lookup2.R#52) 

Error: testthat unit tests failed
Execution halted

See:

http://rgdal.r-forge.r-project.org/articles/PROJ6_GDAL3.html r-spatial/sf#1231 r-spatial/sf#1187 r-spatial/sf#1146 r-spatial/discuss#28

for background. See:

r-spatial/discuss#28 (comment)

for a way of testing fixes in a docker container contributed by Jakub Nowosad.

ateucher commented 4 years ago

Thanks for the heads-up @rsbivand

ateucher commented 4 years ago

@rsbivand did you get these errors with a dev version of sf installed? It looks like that's where the error is occurring (not with sp/rgdal)...

rsbivand commented 4 years ago

Development versions of sp, rgdal, sf (SetFromUserInput branch), run overnigt:

testthat.Rout.fail.sp134rgdal153sf083GDAL304PROJ630.txt

Development versions of sp, rgdal, released sf 0.8-1

testthat.Rout.failsp134rgdal153sf081GDAL304PROJ630.txt

Development versions of sp, rgdal, old released sf 0.8-0:

testthat.Rout.fail.sp134rgdal153sf080GDAL304PROJ630.txt

Released sp, rgdal, old released sf 0.8-0:

testthat.Routsp132rgdal148sf080GDAL303PROJ630.txt

To replicate you'd need 

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

on PROJ 630/GDAL304 or similar. And enjoy diving into testthat code ...

in test-tz_lookup2 l.35 this looks wrong:

  expect_error(tz_lookup_coords(pts, method = "accurate"),
               "It looks like you are trying to get the tz of an sf/sfc or SpatialPoints object") # nolint
Error : It looks like you are trying to get the tz of an sf/sfc or SpatialPoints object! Use tz_lookup() instead.

l. 57

> tz <- tz_lookup_coords(coords[2], coords[1], method = "accurate")
Warning message:
In tz_lookup_accurate.sf(ll_sf) :
  Some points are in areas with more than one time zone defined.These are often disputed areas and should be treated with care.

But running that file line by line gives nothing. I've another 60 packages to handle, so I'll leave this to you - it is projection-related.

rsbivand commented 4 years ago

On my laptop with:

[1] ‘1.3.4’
> packageVersion("rgdal")
[1] ‘1.5.3’
> packageVersion("sf")
[1] ‘0.8.2’
> sf_extSoftVersion()
Error in sf_extSoftVersion() : 
  could not find function "sf_extSoftVersion"
> sf::sf_extSoftVersion()
                 GEOS                  GDAL                proj.4 
              "3.8.0" "3.1.0dev-0d64124ea4"               "7.0.0" 
       GDAL_with_GEOS            USE_PROJ_H 
               "true"                "true" 
> rgdal::rgdal_extSoftVersion()
                 GDAL        GDAL_with_GEOS                  PROJ 
"3.1.0dev-0d64124ea4"                "TRUE"               "7.0.0" 
                   sp 
              "1.3-4"

no problem:

testthat.Rout.txt

So I need to (re)-re-check settings on my desktop.

ateucher commented 4 years ago

Thanks very much for the detail @rsbivand. I haven't been able to replicate the errors using the Nowasad/rspatial_proj6 docker image and development versions of sf, sp, and rgdal, though as you can see I do get warnings related to proj4 strings:

> devtools::test()
Loading lutz
Testing lutz
✓ |  OK F W S | Context
⠏ |  10       | time-zone utilitiesError : foo is not a valid time zone. See ?OlsonNames
✓ |  12       | time-zone utilities [2.5 s]
✓ |  26   10   | tz_lookup works [0.8 s]
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
test-tz_lookup.R:61: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:61: warning: tz_lookup.SpatialPoints works
Discarded datum Unknown_based_on_WGS84_ellipsoid in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:61: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:62: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:62: warning: tz_lookup.SpatialPoints works
Discarded datum Unknown_based_on_WGS84_ellipsoid in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:62: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:65: warning: tz_lookup.SpatialPoints works
Discarded datum Unknown_based_on_GRS80_ellipsoid in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:65: warning: tz_lookup.SpatialPoints works
Discarded datum Unknown_based_on_GRS80_ellipsoid in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:65: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:68: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
✓ |  29       | test-tzlookup2.R [4.3 s]
✓ | 1020       | tz-lookup js edge cases [0.8 s]

══ Results ═══════════════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 8.5 s

OK:       1087
Failed:   0
Warnings: 10
Skipped:  0

> devtools::session_info()
─ Session info ─────────────────────────────────────────────────────────────────────────────────────────────────────
 setting  value                       
 version  R version 3.6.2 (2019-12-12)
 os       Debian GNU/Linux 10 (buster)
 system   x86_64, linux-gnu           
 ui       RStudio                     
 language (EN)                        
 collate  en_US.UTF-8                 
 ctype    en_US.UTF-8                 
 tz       Etc/UTC                     
 date     2020-02-05                  

─ Packages ─────────────────────────────────────────────────────────────────────────────────────────────────────────
 ! package     * version date       lib source                       
   assertthat    0.2.1   2019-03-21 [1] CRAN (R 3.6.2)               
   backports     1.1.5   2019-10-02 [1] CRAN (R 3.6.2)               
   callr         3.4.1   2020-01-24 [1] CRAN (R 3.6.2)               
   class         7.3-15  2019-01-01 [2] CRAN (R 3.6.2)               
   classInt      0.4-2   2019-10-17 [1] CRAN (R 3.6.2)               
   cli           2.0.1   2020-01-08 [1] CRAN (R 3.6.2)               
   colorspace    1.4-1   2019-03-18 [1] CRAN (R 3.6.2)               
   crayon        1.3.4   2017-09-16 [1] CRAN (R 3.6.2)               
   DBI           1.1.0   2019-12-15 [1] CRAN (R 3.6.2)               
   desc          1.2.0   2018-05-01 [1] CRAN (R 3.6.2)               
   devtools      2.2.1   2019-09-24 [1] CRAN (R 3.6.2)               
   digest        0.6.23  2019-11-23 [1] CRAN (R 3.6.2)               
   e1071         1.7-3   2019-11-26 [1] CRAN (R 3.6.2)               
   ellipsis      0.3.0   2019-09-20 [1] CRAN (R 3.6.2)               
   fansi         0.4.1   2020-01-08 [1] CRAN (R 3.6.2)               
   fs            1.3.1   2019-05-06 [1] CRAN (R 3.6.2)               
   ggplot2       3.2.1   2019-08-10 [1] CRAN (R 3.6.2)               
   glue          1.3.1   2019-03-12 [1] CRAN (R 3.6.2)               
   gtable        0.3.0   2019-03-25 [1] CRAN (R 3.6.2)               
   KernSmooth    2.23-16 2019-10-15 [2] CRAN (R 3.6.2)               
   lattice       0.20-38 2018-11-04 [2] CRAN (R 3.6.2)               
   lazyeval      0.2.2   2019-03-15 [1] CRAN (R 3.6.2)               
   lifecycle     0.1.0   2019-08-01 [1] CRAN (R 3.6.2)               
   lubridate     1.7.4   2018-04-11 [1] CRAN (R 3.6.2)               
 P lutz        * 0.3.1   2019-07-19 [?] CRAN (R 3.6.2)               
   magrittr      1.5     2014-11-22 [1] CRAN (R 3.6.2)               
   memoise       1.1.0   2017-04-21 [1] CRAN (R 3.6.2)               
   munsell       0.5.0   2018-06-12 [1] CRAN (R 3.6.2)               
   pillar        1.4.3   2019-12-20 [1] CRAN (R 3.6.2)               
   pkgbuild      1.0.6   2019-10-09 [1] CRAN (R 3.6.2)               
   pkgconfig     2.0.3   2019-09-22 [1] CRAN (R 3.6.2)               
   pkgload       1.0.2   2018-10-29 [1] CRAN (R 3.6.2)               
   prettyunits   1.1.1   2020-01-24 [1] CRAN (R 3.6.2)               
   processx      3.4.1   2019-07-18 [1] CRAN (R 3.6.2)               
   ps            1.3.0   2018-12-21 [1] CRAN (R 3.6.2)               
   R6            2.4.1   2019-11-12 [1] CRAN (R 3.6.2)               
   Rcpp          1.0.3   2019-11-08 [1] CRAN (R 3.6.2)               
   remotes       2.1.0   2019-06-24 [1] CRAN (R 3.6.2)               
   rgdal         1.5-4   2020-02-05 [1] R-Forge (R 3.6.2)            
   rlang         0.4.4   2020-01-28 [1] CRAN (R 3.6.2)               
   rprojroot     1.3-2   2018-01-03 [1] CRAN (R 3.6.2)               
   rstudioapi    0.10    2019-03-19 [1] CRAN (R 3.6.2)               
   scales        1.1.0   2019-11-18 [1] CRAN (R 3.6.2)               
   sessioninfo   1.1.1   2018-11-05 [1] CRAN (R 3.6.2)               
   sf            0.8-2   2020-02-05 [1] Github (r-spatial/sf@edcccbe)
   sp            1.3-4   2020-02-05 [1] Github (rsbivand/sp@38d9ae9) 
   stringi       1.4.5   2020-01-11 [1] CRAN (R 3.6.2)               
   stringr       1.4.0   2019-02-10 [1] CRAN (R 3.6.2)               
   testthat    * 2.3.1   2019-12-01 [1] CRAN (R 3.6.2)               
   tibble        2.1.3   2019-06-06 [1] CRAN (R 3.6.2)               
   units         0.6-5   2019-10-08 [1] CRAN (R 3.6.2)               
   usethis       1.5.1   2019-07-04 [1] CRAN (R 3.6.2)               
   withr         2.1.2   2018-03-15 [1] CRAN (R 3.6.2)               

[1] /usr/local/lib/R/site-library
[2] /usr/local/lib/R/library
[3] /usr/lib/R/library

 P ── Loaded and on-disk path mismatch.

> sf::sf_extSoftVersion()
          GEOS           GDAL         proj.4 GDAL_with_GEOS     USE_PROJ_H 
       "3.8.0"        "3.0.2"        "6.2.1"        "false"         "true" 

> rgdal::rgdal_extSoftVersion()
          GDAL GDAL_with_GEOS           PROJ             sp 
       "3.0.2"        "FALSE"        "6.2.1"        "1.3-4"
rsbivand commented 4 years ago

OK. The warnings are work in progress, and can be muted a bit when the packages get to release. The problem is that in GDAL when reading CRS in files, the exportToProj4() function is used to create the string for CRS and crs objects. As https://gdal.org/api/ogr_srs_api.html#_CPPv416OSRExportToProj420OGRSpatialReferenceHPPc says, the +datum and +towgs84 components are effectively now not to be relied on, so the warnings check the output Proj4 string against the internal CRS representation before export. Going forward, sf and rgdal will export WKT2 representations, so that read files don't degrade (we hope).

ateucher commented 4 years ago

Thanks very much, I really appreciate all the work you're doing to support the transition. I will keep an eye on this!