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.

Sign up for GitHub

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

Jump to bottom

R CMD check failure on Alpine Linux #218

Closed
bastistician opened this issue Feb 15, 2023 · 1 comment · Fixed by #220
Closed

R CMD check failure on Alpine Linux #218

bastistician opened this issue Feb 15, 2023 · 1 comment · Fixed by #220

Comments

@bastistician
Copy link

The test "nested ring depths are correctly exported"

s2/tests/testthat/test-s2-geography.R

Lines 163 to 175 in 76e36ae

# polygon with hole
expect_match(
s2_as_text(
as_s2_geography("MULTIPOLYGON (
((40 40, 20 45, 45 30, 40 40)),
(
(20 35, 10 30, 10 10, 30 5, 45 20, 20 35),
(30 20, 20 15, 20 25, 30 20)
)
)")
),
"\\(20 35, 10 30, 10 10, 30 5, 45 20, 20 35\\), \\(30 20, 20 15, 20 25"
)

fails on Alpine Linux, because the tested s2_as_text() call gives:

MULTIPOLYGON (((40 40.00000000000001, 20 45, 44.99999999999999 30, 40
40.00000000000001)), ((20 35, 9.999999999999998 29.99999999999999,
9.999999999999998 10, 29.99999999999999 4.999999999999999,
44.99999999999999 20, 20 35), (29.99999999999999 20, 20 15, 20 25,
29.99999999999999 20)))

I guess this should use something smaller than the default precision = 16 in s2_as_text.
Using a value of 15 already fixes the test for me. Maybe the default could even be reconsidered?

This is the only failing test (see output below) and I'd appreciate if it could be fixed with the next release.

R CMD check (tests) output 
* checking tests ...
  Running ‘area.R’ [1s/1s]
  Running ‘testthat.R’ [37s/37s]
 [38s/38s] ERROR
Running the tests in 'tests/testthat.R' failed.
Last 13 lines of output:
  
  [ FAIL 1 | WARN 0 | SKIP 0 | PASS 1072 ]
  
  ══ Failed tests ════════════════════════════════════════════════════════════════
  ── Failure ('test-s2-geography.R:164'): nested ring depths are correctly exported ──
  s2_as_text\(as_s2_geography\("MULTIPOLYGON \(\\n        \(\(40 40, 20 45, 45 30, 40 40\)\),\\n        \(\\n          \(20 35, 10 30, 10 10, 30 5, 45 20, 20 35\),\\n          \(30 20, 20 15, 20 25, 30 20\)\\n        \)\\n      \)"\)\) does not match "\\(20 35, 10 30, 10 10, 30 5, 45 20, 20 35\\), \\(30 20, 20 15, 20 25".
  Actual value: "MULTIPOLYGON \(\(\(40 40\.00000000000001, 20 45, 44\.99999999999999 30, 40 40\.00000000000001\)\), \(\(20 35, 9\.999999999999998 29\.99999999999999, 9\.999999999999998 10, 29\.99999999999999 4\.999999999999999, 44\.99999999999999 20, 20 35\), \(29\.99999999999999 20, 20 15, 20 25, 29\.99999999999999 20\)\)\)"
  Backtrace:
      ▆
   1. └─testthat::expect_match(...) at test-s2-geography.R:164:2
   2.   └─testthat:::expect_match_(...)
  
  [ FAIL 1 | WARN 0 | SKIP 0 | PASS 1072 ]
  Error: Test failures
  Execution halted

FWIW, the other s2_as_text() test using a "polygon with a hole in a hole" gives similar output but does not fail the expectation as the matched element is reproduced without numerical fuzz even with the default precision = 16:

MULTIPOLYGON (((40 40.00000000000001, 20 45, 44.99999999999999 30, 40
40.00000000000001)), ((20 35, 9.999999999999998 29.99999999999999,
9.999999999999998 10, 29.99999999999999 4.999999999999999,
44.99999999999999 20, 20 35), (29.99999999999999 20, 20 15, 20 25,
29.99999999999999 20)), ((27 21, 21 21, 21 16, 27 21)))
sessionInfo() 
R Under development (unstable) (2023-02-09 r83794)
Platform: x86_64-pc-linux-musl (64-bit)
Running under: Alpine Linux v3.17

Matrix products: default
BLAS:   /home/smeyer/QA/R-83794/build/lib/libRblas.so 
LAPACK: /home/smeyer/QA/R-83794/build/lib/libRlapack.so;  LAPACK version 3.11.0

locale:
[1] C.UTF-8 C       C       C       C       C      

time zone: Europe/Berlin
tzcode source: internal
@paleolimbot
Copy link
Collaborator

Thans for reporting! I'll drop the precision for that test - thanks for the suggestion.

@paleolimbot paleolimbot mentioned this issue Feb 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Less strict test for polygon hole round trip r-spatial/s2
2 participants
@bastistician @paleolimbot