DOI-USGS / intersectr

See official repository here: https://code.usgs.gov/water/intersectr
Creative Commons Zero v1.0 Universal
7 stars 5 forks source link

fix bug in latest changes. #14

Closed dblodgett-usgs closed 5 years ago

dblodgett-usgs commented 5 years ago

Solution to deal with the date line is pretty far out there -- but date line spanning datasets (the Aleutians) are going to be pretty rare.

dblodgett-usgs commented 5 years ago

Wow! So that travis run around turned out to be due to using UTM zone 1 projection (which worked locally) but bombed travis. @edzer, were you aware that something is funky with the UTM projection handling behind st_transform? I tried both the proj format and the numeric epsg... This fixed the failing test (using a non UTM projection): https://github.com/USGS-R/intersectr/pull/14/commits/983a782a303a42c4fd2c0c585c404bdef8b7e680#diff-41899fd8edb03e22fc322422a4fd4156L248

Travis logs were awesome...

  > test_check("intersectr")
  ── 1. Failure: crossing date line more (@test_create_cell_geometry.R#103)  ─────
  unknown failure

  ══ testthat results  ═══════════════════════════════════════════════════════════
  OK: 57 SKIPPED: 0 WARNINGS: 31 FAILED: 1
  1. Failure: crossing date line more (@test_create_cell_geometry.R#103) 

  Error: testthat unit tests failed
  Execution halted
* DONE
Status: 1 ERROR, 2 NOTEs
edzer commented 5 years ago

I tried your tests:

library(sf)
 geom <- matrix(c(-173.1318, -179.1185,
                -179.1336, -179.1488, 179.226, 179.2137, 173.6277, 173.3859,
               173.317, 173.3046, 173.2945, 172.3714, 172.3603, 172.3528, 172.3477,
                   172.3462, 172.3474, 172.3506, 172.3565, 172.3645, -173.1305,
                   -152.0446, -151.5654, -151.5436, -151.5136, -151.512, -151.5105,
                   -151.6557, -151.6715, -152.7324, -154.3472, -154.3605, -159.1322,
                   -159.1487, -159.1973, -159.21, -159.4758, -159.489, -159.506,
                   -169.2089, -169.2415, -173.1318, 51.95922, 51.15853, 51.15695,
                   51.1572, 51.29705, 51.29861, 52.29857, 52.33802, 52.35366, 52.35788,
                   52.36279, 52.88758, 52.89557, 52.90398, 52.91396, 52.92348, 52.93215,
                   52.93974, 52.94769, 52.95491, 60.73862, 64.41658, 64.37961, 64.37583,
                   64.35263, 64.34964, 64.31368, 63.02242, 62.93349, 60.74787, 58.45999,
                   58.44583, 54.87275, 54.85934, 54.82705, 54.82081, 54.71403, 54.70982,
                   54.70642, 52.71876, 52.7118, 51.95922), ncol = 2) %>%
  list() %>%
  st_polygon() %>%
  st_sfc(crs = 4326)

geom
plot(geom, axes = TRUE)
(geom <- st_convex_hull(st_union(st_transform(geom, 32601))))
plot(geom, axes = TRUE)
(geom <- st_convex_hull(st_union(st_transform(geom, 3832))))
plot(geom, axes = TRUE)

am see sane geometries for both epsg 32601 and 3832, unlike 4326 (as expected). Could it be the cause that travis is still running PROJ 4.8.0 (released in 2012)? What did the travis errors say?

dblodgett-usgs commented 5 years ago

Travis just said Execution halted -- I guess the PROJ UTM stuff might have changed since 2012? 🤷‍♂

edzer commented 5 years ago

PROJ sometimes returns NaN coordinates that have caused headaches in sf. @rsbivand do you have any recollections of this in relation to UTM zone 1 and antimeridian crossing polygons, for PROJ 4.8.0?

dblodgett-usgs commented 5 years ago

ehhh... nevermind. throw it in the bin of unreproducible travis issues...

https://github.com/dblodgett-usgs/intersectr/commit/a19e00bddbb5 Passes... https://travis-ci.org/dblodgett-usgs/intersectr/builds/525427522