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

GEOS version sensitivity #50

Closed
rsbivand opened this issue Jul 4, 2022 · 7 comments
Closed

GEOS version sensitivity #50

rsbivand opened this issue Jul 4, 2022 · 7 comments
Labels

Comments

@rsbivand
Copy link

rsbivand commented Jul 4, 2022

00check.log
testthat.Rout.fail.log

for GEOS 3.11.0 show a failure probably because of fixes in GEOS to the NG topology engine re-ordering the coordinates of the polygon returned by sf::st_union(x)):

> st_coordinates(st_geometry(y))
        X   Y L1 L2
 [1,]  50  50  1  1
 [2,]   0  50  1  1
 [3,]  75 125  1  1
 [4,]   0 125  1  1
 [5,] 100 250  1  1
 [6,] 200 125  1  1
 [7,] 125 125  1  1
 [8,] 200  50  1  1
 [9,] 150  50  1  1
[10,] 200   0  1  1
[11,]   0   0  1  1
[12,]  50  50  1  1
> st_coordinates(st_geometry(y2))
        X   Y L1 L2
 [1,] 200   0  1  1
 [2,]   0   0  1  1
 [3,]  50  50  1  1
 [4,]   0  50  1  1
 [5,]  75 125  1  1
 [6,]   0 125  1  1
 [7,] 100 250  1  1
 [8,] 200 125  1  1
 [9,] 125 125  1  1
[10,] 200  50  1  1
[11,] 150  50  1  1
[12,] 200   0  1  1

Please adapt equality test - perhaps drop or compare areas.

@jeffreyhanson
Copy link
Collaborator

Hi @rsbivand,

Thank you very much for letting me know. I'll try and fix this as soon as I can. Just so that I'm aware of the timelines, how soon does the fix for wdpar need to be on CRAN?

@rsbivand
Copy link
Author

rsbivand commented Jul 4, 2022

Thanks! Timing unpredictable, GEOS 3.11.0 was just released, and CRAN waits for Debian and Fedora packages, so it depends on the upstream package maintainers' urgency or not. I have 3.11.0 installed, so please ping me when you have a fix that needs trying.

@jeffreyhanson
Copy link
Collaborator

Ok - yeah, that would be extremely helpful, I'll let you know when I've got a (potential) fix ready - thanks!

jeffreyhanson added a commit that referenced this issue Jul 5, 2022
@jeffreyhanson
Copy link
Collaborator

@rsbivand, I've just pushed a fix to the fix-geos branch. When you get a chance, could you please try it out and see if that fixes it? I've updated the test to use st_equals() which returns TRUE when comparing the coordinates you posted earlier?

@rsbivand
Copy link
Author

rsbivand commented Jul 5, 2022

Logs for _SP_EVOLUTION_STATUS=2 _R_CHECK_FORCE_SUGGESTS_=FALSE R CMD check wdpar_1.3.2.4.tar.gz and _SP_EVOLUTION_STATUS_=2 R CMD build --no-build-vignettes wdpar-fix-geos:

00check.log

testthat.Rout.log

Looks OK! When the evolution process has moved a bit more forward, sp will be flipped from evolution status 0 using rgdal under the hood to evolution status 2 using sf under the hood, so the environment variable will cease to be needed.

@jeffreyhanson
Copy link
Collaborator

jeffreyhanson commented Jul 7, 2022

Awesome! Thank you for checking that. I'll try and submit an update to CRAN sometime later today or tomorrow.

@jeffreyhanson
Copy link
Collaborator

Just to let you know, I submitted the update yesterday and received notification that it's on it's way to CRAN.

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

No branches or pull requests

2 participants