SOgg combine problem

[mdsumner]

test fail needs attention

    ## gg-ify p together with pm
   # pg <- SOgg(p, pm) ## no warning this time
   #    expect_s3_class(pg, "SOmap_gg")
   #    expect_identical(sort(names(pg)), c("axis_labels", "bathy", "bathy_legend", "border", "ccamlr_planning_domains", "ccamlr_ssmu", "ccamlr_ssru", "ccamlr_statistical_areas", "coastline", "coord", "eez", "ice", "init", "iwc", "mpa", "plot_sequence", "projection", "research_blocks", "scale_fill", "sprfmo_research_blocks", "straight", "target", "theme", "trim"))
   #    expect_s3_class(plot(pg), "ggplot")

    ## things in pg should be of expected classes
    # expect_is(pg$target, "Raster")
    # expect_is(pg$plot_sequence, "character")
    # expect_is(pg$projection, "character")
    # expect_is(pg$straight, "logical")
    # expect_is(pg$trim, "numeric")
    # ## everything else in p should be a list of SO_plotter objects
    # for (cmp in setdiff(names(pg), c("target", "plot_sequence", "projection", "straight", "trim"))) {
    #     expect_is(pg[[cmp]], "list")
    #     expect_true(all(vapply(pg[[cmp]], inherits, "SO_plotter", FUN.VALUE = TRUE)))
    # }

[raymondben]

think that's one of mine, will look tomorrow

[raymondben]

@mdsumner These tests all run fine for me, what "topology" error should I be seeing?

[Maschette]

I am getting 2 test fails on this branch:

-- 1. Failure: adding data to a plot is fine (@test-SOmap.R#27)  --------------------------
`SOplot(...)` did not produce any warnings.

-- 2. Error: SOgg returns a ggplot object (@test-ggplot.R#29)  ----------------------------
Evaluation error: TopologyException: Input geom 1 is invalid: Ring Self-intersection at or near point -2606239.1226274539 3253575.9021713207 at -2606239.1226274539 3253575.9021713207.
Backtrace:
  1. SOmap::SOgg(p, pm)
  3. base::lapply(x, SOgg)
  4. SOmap:::FUN(X[[i]], ...)
  5. SOmap:::SOgg_management(x)
  8. SOmap:::apply_buf(sf::st_as_sf(x$ccamlr_ssmu$main$plotargs$x))
 12. sf:::st_intersection.sf(buf, thing)
 14. sf:::geos_op2_geom("intersection", x, y)
 16. sf:::CPL_geos_op2(op, st_geometry(x), st_geometry(y))

[Maschette]

with the second one it is line 29 pg <- SOgg(p, pm) ## no warning this time which is throwing the error for me.

[raymondben]

Ta. The first one is no longer throwing whatever warning it used to (probably warning about "no projection specified, assuming longlat" at a guess) - I've just removed the expect_warning. The second one is failing at the apply_buf call, which in turn is calling sf:::st_intersection.sf. One for @mdsumner?

[mdsumner]

all good I'll try to get to this soon

[mdsumner]

This code is extremely ambitious, I just don't think we can expect stability with this - do we have to have such complexity?

[mdsumner]

It comes down to this

apply_buf(sf::st_as_sf(x$ccamlr_ssmu$labels$plotargs$x))
 Error in CPL_geos_op2(op, st_geometry(x), st_geometry(y)) : 
  Evaluation error: TopologyException: Input geom 1 is invalid: Ring Self-intersection at or near point -2606239.1226274539 3253575.9021713207 at -2606239.1226274539 3253575.9021713207. 

We can fix with the egregious st_buffer(, 0) hack. ergh

The test takes a long time too, it might be best if we turn everything into sf and buffer=0 it upfront. But blewegh

[raymondben]

The time for this test has blown out massively, it did not used to be this slow. It's because of the coastline complexity buried in the CCAMLR domains management layers, which for whatever reason now seems to be much slower than before (changes to sf?). Do we need to go back to https://github.com/AustralianAntarcticDivision/SOmap/issues/17 and take the coastline out of that layer?

[mdsumner]

It's probably enough to reduce the detail with rmapshaper

[mdsumner]

i.e.

sf::as_Spatial(rmapshaper::ms_simplify(sf::st_as_sf(x), 0.1, keep_shapes = TRUE))  ## keep 10% detail

[raymondben]

simplified data in https://github.com/AustralianAntarcticDivision/SOmap/commit/2f504df96bdd58e9eddcecbdc97158830c6e170f Ultimately I'd still like to remove the coastline from layers other than actual coastline layers, but that's a separate issue for later