I'm trying to update the geobr pacakge on CRAN following the new crs style of sf 0.9-0. I've came across this problem when running my package tests. The package passes all tests on Windows, but it fails on Linux Debian and I believe this is because sf::st_crs(mysf)$proj4string is having a different behavior on Windows and Linux.
The code sf::st_crs(mysf)$proj4string seems to return slightly different results on Windows and Linux Debian. Here is what I get:
Windows: "+proj=longlat +ellps=GRS80 +towgs84=0,0,0,0,0,0,0 +no_defs "
Linux Debian: "+proj=longlat +ellps=GRS80 +towgs84=0,0,0,0,0,0,0 +no_defs"
Mind you that, on windows, there is blank space between the last character and the quote sign " while there's no blank space on Linux.
Here is the code used in the test.
# read data
test_sf <- geobr::read_amazon(showProgress = F)
testthat::expect_equal(sf::st_crs(test_sf)$proj4string, "+proj=longlat +ellps=GRS80 +towgs84=0,0,0,0,0,0,0 +no_defs ")
````
The package passes all tests on Windows, but it returns this error on Linux Debian:
And the error:
Flavor: r-devel-linux-x86_64-debian-gcc
Check: tests, Result: ERROR
Running 'testthat.R' [2s/7s]
Running the tests in 'tests/testthat.R' failed.
Complete output:
library(testthat)
library(geobr)
Loading required namespace: sf
library(sf)
Linking to GEOS 3.8.0, GDAL 3.0.4, PROJ 7.0.0test_check("geobr")
โโ 1. Failure: read_amazon (@test-read_amazon.R#23) โโโโโโโโโโโโโโโโโโโโโโโโโโโ
sf::st_crs(test_sf)$proj4string not equal to "+proj=longlat +ellps=GRS80 +towgs84=0,0,0,0,0,0,0 +no_defs ".
1/1 mismatches
x[1]: "+proj=longlat +ellps=GRS80 +towgs84=0,0,0,0,0,0,0 +no_defs"
y[1]: "+proj=longlat +ellps=GRS80 +towgs84=0,0,0,0,0,0,0 +no_defs "
โโ testthat results โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโ
[ OK: 9 | SKIPPED: 23 | WARNINGS: 0 | FAILED: 1 ]
1. Failure: read_amazon (@test-read_amazon.R#23)
Error: testthat unit tests failed
Execution halted
```
The general recommendation is to move away from using PROJ4 strings to characterize coordinate reference systems. I don't think that problems of this kind will be solved.
Thank you for the clarification. In this case, I'll change the test to be testthat::expect_equal(sf::st_crs(test_sf)$epsg, 4674). this should solve the problem.
Do read https://www.r-spatial.org/r/2020/03/17/wkt.html and note that from
0.9-0, the "crs" object has neither epsg nor proj4string components. Why do
you need to test for equality at all?
Roger Bivand
Falsensvei 32
5063 Bergen
ons. 25. mar. 2020, 17.37 skrev Rafael H M Pereira <[email protected]
:
Thank you for the clarification. In this case, I'll change the test to be testthat::expect_equal(sf::st_crs(test_sf)$epsg,
4674). this should solve the problem.โ
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/r-spatial/sf/issues/1318#issuecomment-603948323, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/ACNZ3BCSD5P5YJORYZA257DRJIXNDANCNFSM4LTR4REQ
.
@rsbivand st_crs(test_sf)$epsg extracts the epsg through a GDAL call. @rafapereirabr that may or may not work - hard to trust in general I have found.
I know that it extracts from proj.db, but across platforms and PROJ
versions, such tests are very fragile, and should probably be avoided
completely until 2022/3. Simply comment out the test. Tests in plain tests,
not testthat are much easier to parse too, so a maintainer may use a
regular test, and always read results carefully. Lots of the testthat tests
are not needed and are badly written.
Roger Bivand
Falsensvei 32
5063 Bergen
ons. 25. mar. 2020, 17.53 skrev Edzer Pebesma notifications@github.com:
@rsbivand https://github.com/rsbivand st_crs(test_sf)$epsg extracts the
epsg through a GDAL call. @rafapereirabr
https://github.com/rafapereirabr that may or may not work - hard to
trust in general I have found.โ
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/r-spatial/sf/issues/1318#issuecomment-603957309, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/ACNZ3BHTIN4V4TPP3C2MLDLRJIZHFANCNFSM4LTR4REQ
.
Reading this with interest. Not directly related to the issue but unsure what this means:
Tests in plain tests
For people who learned package development recently and who have read https://r-pkgs.org/ testthat may be the only testing framework they know about.
Agree that many tests are not needed or badly written. I used just to put tests in the @examples section but am not sure what the other options are.
Tests in examples also work, but one needs to read the output. Standard
tests are just a short scripts in the tests directory, xx.R, with an
xx.Rout.save file with expected output, copied from the check output
xx.Rout after the first check run.
https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Package-subdirectories,
tests subsection. Differences do not trigger CRAN errors, so are for
maintainers who read output. Admittedly, some should be fatal errors, and
sf and rgdal at the moment spew out differences, because of the crs
transition, but very many revdep failures are caused by overtesting and/or
wrong tests.
Roger Bivand
Falsensvei 32
5063 Bergen
ons. 25. mar. 2020, 18.03 skrev Robin notifications@github.com:
Reading this with interest. Not directly related to the issue but unsure
what this means:Tests in plain tests
For people who learned package development recently and who have read
https://r-pkgs.org/ testthat may be the only testing framework they know
about.Agree that many tests are not needed or badly written. I used just to put
tests in the @examples section but am not sure what the other options are.โ
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/r-spatial/sf/issues/1318#issuecomment-603963649, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/ACNZ3BBNX56ZFXXEQKYFYHLRJI2PLANCNFSM4LTR4REQ
.
very many revdep failures are caused by overtesting and/or
wrong tests.
I agree with this - as a result of this transition I have (I think) removed all tests from rmapshaper that compare crs attributes - they are fragile and are the job of sf/rgdal to test, not the revdeps (in most cases anyway).
Hi all, thank you for sharing your thoughts on this.
In my case, geobr is a package to download official spatial data sets of Brazil For various reasons, we have to process the original data sets and save them to our server, from where they're downloaded. So in my particular case, I'd like to keep a test like testthat::expect_equal(sf::st_crs(test_sf)$epsg, 4674). to make sure all of our data sets have gone through the correct data preparation process.
Please use a simple test that alerts you and you only if the result is
unexpected. Do not rely on testthat, and do not cause unneeded errors in
revdep tests. Really, we are run off our feet handling these cases which
are not errors in sf or rgdal, but may simply come from updates in oroj.db
that are not under our control. Please do not make our work harder with
fatal tests that should never be fatal, but should only alert you as
maintainer to an anomaly. You expect that sf and rgdal promise unchanging
output. We cannot promise that.
Roger Bivand
Falsensvei 32
5063 Bergen
ons. 25. mar. 2020, 19.17 skrev Rafael H M Pereira <[email protected]
:
Hi all, thank you for sharing your thoughts on this.
In my case, geobr is a package to download official spatial data sets of
Brazil For various reasons, we have to process the original data sets and
save them to our server, from where they're downloaded. So in my particular
case, I'd like to keep a test like testthat::expect_equal(sf::st_crs(test_sf)$epsg,
4674). to make sure all of our data sets have gone through the correct
data preparation process.โ
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/r-spatial/sf/issues/1318#issuecomment-604003541, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/ACNZ3BBASM2EESXOIKH2ZMTRJJDBZANCNFSM4LTR4REQ
.
Hi Roger. I understand your point and I very much appreciate the work you guys put into this. We can close this issue.
Heads-up @rafapereirabr and @rsbivand and anyone else watching, I've just tried the minimal tinytest package. The framework seems really good and, as Roger recommended, treats test fails as data not as errors. Recommended: https://github.com/markvanderloo/tinytest/tree/master/pkg
Thanks for the heads up, @Robinlovelace !
Most helpful comment
Heads-up @rafapereirabr and @rsbivand and anyone else watching, I've just tried the minimal
tinytestpackage. The framework seems really good and, as Roger recommended, treats test fails as data not as errors. Recommended: https://github.com/markvanderloo/tinytest/tree/master/pkg