Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aqp failing with development sp and rgdal/PROJ6+GDAL3 #109

Closed
rsbivand opened this issue Jan 9, 2020 · 8 comments
Closed

aqp failing with development sp and rgdal/PROJ6+GDAL3 #109

rsbivand opened this issue Jan 9, 2020 · 8 comments

Comments

@rsbivand
Copy link

rsbivand commented Jan 9, 2020

test_check("aqp")
── 1. Failure: SPC spatial operations (@test-SPC-objects.R#153) ──────────────
sp::proj4string(sp1) not equal to "+proj=longlat +datum=NAD83 +ellps=GRS80 +towgs84=0,0,0".
1/1 mismatches
x[1]: "+proj=longlat +ellps=GRS80 +towgs84=0,0,0,0,0,0,0 +no_defs"
y[1]: "+proj=longlat +datum=NAD83 +ellps=GRS80 +towgs84=0,0,0"

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.

@dylanbeaudette
Copy link
Member

Thanks Roger. I haven't been keeping-up on the PROJ6 / GDAL3 news. Based on my initial reading of the first link, proj4 strings are being phased out in favor of more precise CRS notation. A couple of questions:

  • should the SoilProfileCollection class switch to a new interface for getting / setting CRS, e.g. away from proj4string()?
  • what do you recommend for folks on either side of the divide? Many aqp users (US Dept. Agriculture) are sadly hampered by IT and cannot use the latest version of R.

@rsbivand
Copy link
Author

rsbivand commented Jan 9, 2020

Wrt. use of proj4string(): for sp workflows, jury out. I feel keeping backward compatibility matters, so the function will still just return the "projargs" slot of the "CRS" object. If you replace it by slot(<Spatial>, "proj4string") for "Spatial" objects, you also get the comment with the WKT2 string, which is what we will be using for operations where available (in sp workflows). sf/stars are S3 not S4, so class definitions are not fixed, and changing the sf "crs" class doesn't break serialised (saved as RData) objects.

You might also consider transitioning from the sp to the sf workflow, as maintenance of rgdal and rgeos is not something I can commit to deep into retirement, I'm afraid.

For US government users, the future is coming fast, see for example:

https://www.esri.com/about/newsroom/arcuser/moving-from-static-spatial-reference-systems-in-2022/

The version of R, or CRAN packages is less important than the versions of PROJ and GDAL, with the break coming between PROJ < 6 and GDAL < 3, and PROJ >= 6 and GDAL >= 3. From PROJ 7, a CDN will also make transformation grids available on-the-fly, but we are not there yet (I posted on R-sig-geo to elicit responses from institutional users, but got no response about whether they need to control which grids are used). We will probably see many Linux systems (including cloud nodes) shifting to recent PROJ and GDAL binaries; for CRAN Windows binaries, I think we'd like to shift once things look moderately stable, same for CRAN OSX.

@dylanbeaudette
Copy link
Member

Thanks Roger, this is excellent advice. I'm thinking about transitioning over to sf objects / methods for management of the spatial data contained within SoilProfileCollection objects. I've used sf and like it, but it is hard moving on from sp after nearly 15 years of productive work based on those classes / methods.

We are planning some major changes to the SoilProfileCollection object so now is a good time to make the switch. What are your thoughts on transitioning to S3 vs. staying with S4?

@rsbivand
Copy link
Author

I still feel that the arguements for S4 hold when the aim is to structure the data in known forms. Interoperability (including S3 in S4 class definitions) has I think been improving. Bioconductor I think remains largely S4, and they may well have tools for making interoperability more fluent. I guess you'd benefit from trying out prototypes. With S3, it easier to navigate breaking changes, so that in sp we can't change the "CRS" object without breaking serialised objects (RData or rds), but sf can check for NULL list members (say in "crs" objects.

@dylanbeaudette
Copy link
Member

Thanks for the feedback. Until we get aqp figured out, are we still breaking upstream sp and rgdal?

@rsbivand
Copy link
Author

No, aqp is being broken by sp and rgdal which are adapting to meet new requirements in GDAL and PROJ. So when they reach CRAN, CRAN will ask for the problem to ve fixed (same problem with sf, because everybody has to adapt to PROJ6/GDAL3. The same error was present on Monday last.

This problem - there may be others, is that the test on line 153 in the named file fails because the proj4string gets updated differently on being passed through GDAL/PROJ now, dropping the +datum= term among other things. In any case, maybe the sp1 object had a CRS before, so overwriting it was a bit brutal.

@dylanbeaudette
Copy link
Member

Thanks for the clarification. I'm curious if the aqp on CRAN is still failing this test. I removed the +datum= section from the proj4 string a while back.

@rsbivand
Copy link
Author

rsbivand commented Feb 2, 2020

CRAN is OK: https://cran.r-project.org/web/checks/check_results_aqp.html. The failure I reported for the development version of sp/rgdal with GDAL3/PROJ6 was with aqp 1.18-1, not released 1.19. I'll be re-running the checks this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants