joelgombin / concaveman

A very fast 2D concave hull algorithm
https://joelgombin.github.io/concaveman/
66 stars 7 forks source link

upcoming sf breaks concaveman checks #6

Closed edzer closed 4 years ago

edzer commented 6 years ago

With:

> ### Name: concaveman
> ### Title: concaveman: A very fast 2D concave hull algorithm.
> ### Aliases: concaveman concaveman-package concaveman concave .... [TRUNCATED] 

> polygons <- concaveman(points)

> plot(points)

> plot(polygons, add = TRUE)
Error in if (cl %in% c("integer", "numeric")) stripchart(x1, ...) else plot(x1,  : 
  the condition has length > 1
> traceback()
6: plot.data.frame(polygons, add = TRUE)
5: plot(polygons, add = TRUE) at concaveman-Ex.R#37
4: eval(ei, envir)
3: eval(ei, envir)
2: withVisible(eval(ei, envir))
1: source("rdepends_concaveman.Rcheck/concaveman-Ex.R", , T)

If I later on do something with points, things run fine. Maybe load sf before running the example? Let me know if you need help.

joelgombin commented 5 years ago

Thanks @edzer and sorry for answering such a long time after you opened the issue! With which version of sf did you try this? If I try on my side (with sf 0.7-2,

plot(polygons, add = TRUE)

throws a warning, not an error. Also, no polygon is drawn. So I guess the issue might be with the plot.sf method? If I only do

plot(polygons)

I get the same warning but the polygon is actually drawn. What do you think?

edzer commented 5 years ago

I think these errors can be triggers when you set environment variables _R_CHECK_LENGTH_1_LOGIC2_=package:_R_CHECK_PACKAGE_NAME_,abort,verbose, or e.g. try with winbuilder.

joelgombin commented 5 years ago

Actually there's something strange. If I don't load sf, the output of concaveman is of class data.frame (where I would expect sf - data.frame). If I load sf before running concaveman, the output is of class sf...

edzer commented 5 years ago

reprex?

joelgombin commented 5 years ago
library(concaveman)

class(points)
#> [1] "sf"         "data.frame"
polygon <- concaveman(points)
class(polygon)
#> [1] "data.frame"

library(sf)
#> Linking to GEOS 3.6.2, GDAL 2.2.3, PROJ 4.9.3

polygon <- concaveman(points)
class(polygon)
#> [1] "sf"         "data.frame"

Created on 2019-01-10 by the reprex package (v0.2.1)

Session info ``` r devtools::session_info() #> ─ Session info ────────────────────────────────────────────────────────── #> setting value #> version R version 3.4.4 (2018-03-15) #> os Ubuntu 18.04.1 LTS #> system x86_64, linux-gnu #> ui X11 #> language (EN) #> collate fr_FR.UTF-8 #> ctype fr_FR.UTF-8 #> tz Europe/Paris #> date 2019-01-10 #> #> ─ Packages ────────────────────────────────────────────────────────────── #> package * version date lib source #> assertthat 0.2.0 2017-04-11 [1] CRAN (R 3.4.1) #> backports 1.1.3 2018-12-14 [1] CRAN (R 3.4.4) #> bindr 0.1.1 2018-03-13 [1] CRAN (R 3.4.3) #> bindrcpp * 0.2.2 2018-03-29 [1] CRAN (R 3.4.3) #> callr 3.1.1 2018-12-21 [1] CRAN (R 3.4.4) #> class 7.3-15 2019-01-01 [1] CRAN (R 3.4.4) #> classInt 0.3-1 2018-12-18 [1] CRAN (R 3.4.4) #> cli 1.0.1 2018-09-25 [1] CRAN (R 3.4.4) #> concaveman * 1.0.0.9000 2018-06-11 [1] local #> crayon 1.3.4 2017-09-16 [1] CRAN (R 3.4.1) #> curl 3.2 2018-03-28 [1] CRAN (R 3.4.4) #> DBI 1.0.0 2018-05-02 [1] CRAN (R 3.4.3) #> desc 1.2.0 2018-05-01 [1] CRAN (R 3.4.3) #> devtools 2.0.1 2018-10-26 [1] CRAN (R 3.4.4) #> digest 0.6.18 2018-10-10 [1] CRAN (R 3.4.4) #> dplyr 0.7.8 2018-11-10 [1] CRAN (R 3.4.4) #> e1071 1.7-0 2018-07-28 [1] CRAN (R 3.4.4) #> evaluate 0.12 2018-10-09 [1] CRAN (R 3.4.4) #> fs 1.2.6 2018-08-23 [1] CRAN (R 3.4.4) #> glue 1.3.0 2018-07-17 [1] CRAN (R 3.4.4) #> highr 0.7 2018-06-09 [1] CRAN (R 3.4.4) #> htmltools 0.3.6 2017-04-28 [1] CRAN (R 3.4.1) #> jsonlite 1.6 2018-12-07 [1] CRAN (R 3.4.4) #> knitr 1.21 2018-12-10 [1] CRAN (R 3.4.4) #> magrittr 1.5 2014-11-22 [1] CRAN (R 3.4.1) #> memoise 1.1.0 2017-04-21 [1] CRAN (R 3.4.1) #> pillar 1.3.1 2018-12-15 [1] CRAN (R 3.4.4) #> pkgbuild 1.0.2 2018-10-16 [1] CRAN (R 3.4.4) #> pkgconfig 2.0.2 2018-08-16 [1] CRAN (R 3.4.4) #> pkgload 1.0.2 2018-10-29 [1] CRAN (R 3.4.4) #> prettyunits 1.0.2 2015-07-13 [1] CRAN (R 3.4.4) #> processx 3.2.1 2018-12-05 [1] CRAN (R 3.4.4) #> ps 1.3.0 2018-12-21 [1] CRAN (R 3.4.4) #> purrr 0.2.5 2018-05-29 [1] CRAN (R 3.4.3) #> R6 2.3.0 2018-10-04 [1] CRAN (R 3.4.4) #> Rcpp 1.0.0 2018-11-07 [1] CRAN (R 3.4.4) #> remotes 2.0.2 2018-10-30 [1] CRAN (R 3.4.4) #> rlang 0.3.0.1 2018-10-25 [1] CRAN (R 3.4.4) #> rmarkdown 1.11 2018-12-08 [1] CRAN (R 3.4.4) #> rprojroot 1.3-2 2018-01-03 [1] CRAN (R 3.4.3) #> sessioninfo 1.1.1 2018-11-05 [1] CRAN (R 3.4.4) #> sf * 0.7-2 2018-12-20 [1] CRAN (R 3.4.4) #> stringi 1.2.4 2018-07-20 [1] CRAN (R 3.4.4) #> stringr 1.3.1 2018-05-10 [1] CRAN (R 3.4.3) #> testthat 2.0.1 2018-10-13 [1] CRAN (R 3.4.4) #> tibble 1.4.2 2018-01-22 [1] CRAN (R 3.4.3) #> tidyselect 0.2.5 2018-10-11 [1] CRAN (R 3.4.4) #> units 0.6-2 2018-12-05 [1] CRAN (R 3.4.4) #> usethis 1.4.0 2018-08-14 [1] CRAN (R 3.4.4) #> V8 1.5 2017-04-25 [1] CRAN (R 3.4.1) #> withr 2.1.2 2018-03-15 [1] CRAN (R 3.4.3) #> xfun 0.4 2018-10-23 [1] CRAN (R 3.4.4) #> yaml 2.2.0 2018-07-25 [1] CRAN (R 3.4.4) #> #> [1] /home/joel/R/x86_64-pc-linux-gnu-library/3.4 #> [2] /usr/local/lib/R/site-library #> [3] /usr/lib/R/site-library #> [4] /usr/lib/R/library ```
edzer commented 5 years ago

That is because the summarise.sf method is only picked up (i.e., used) when sf is loaded. Without, you'll go through summarise.data.frame, provided by dplyr, which won't create sf objects.

joelgombin commented 5 years ago

Right. But I can't force the call to summarise.sf, can I ?

edzer commented 5 years ago

Your package needs to import it (can't see it in the NAMESPACE).

joelgombin commented 5 years ago

OK. But I wanted to avoid importing sf if it is not needed, and my understanding was that for S3 methods I didn't need to. See what Hadley says:

S3 generics are just functions, so the same rules for functions apply. S3 methods always accompany the generic, so as long as you can access the generic (either implicitly or explicitly), the methods will also be available. In other words, you don’t need to do anything special for S3 methods. As long as you’ve imported the generic, all the methods will also be available.

Am I getting it wrong?

edzer commented 5 years ago

But how would your package then spontaneously jump to sf::summarise.sf without even knowing it exists?

You can leave it in Suggests: but then need to do sth like this:

    if (!requireNamespace("sf", quietly = TRUE))
        stop("sf required: install that first") # nocov

        sf::summarise( ...)

so that your code imports the sf namespace on the fly, when needed.

joelgombin commented 4 years ago

BTW I've made sf a strong dependency to avoid this issue.