rOpenGov / sorvi

Algorithms for Finnish open goverment data
http://ropengov.github.io/sorvi
Other
13 stars 4 forks source link

ggplot2 maps on CRAN #27

Open pitkant opened 10 months ago

pitkant commented 10 months ago

sorvi 0.8.21 was published yesterday (2023-08-22) on CRAN. However, in the package vignette there seems to be some problem with drawing maps with ggplot2:

library(ggplot2)
map1931 <- get_municipalities(year = 1931)
ggplot(map1931) + geom_sf()
#> Warning in grid.Call.graphics(C_path, x$x, x$y, index, switch(x$rule, winding =
#> 1L, : Path drawing not available for this device
[...]
#> Warning in grid.Call.graphics(C_path, x$x, x$y, index, switch(x$rule, winding =
#> 1L, : Path drawing not available for this device

This is not a unique warning, see e.g. https://github.com/r-spatial/sf/issues/1395 and https://github.com/ggseg/ggseg/issues/85. Additionally Warning in grid.Call.graphics warnings can be related to something else, such as "semi-transparency is not supported on this device: reported only once per page" in the case of sotkanet README: https://cloud.r-project.org/web/packages/sotkanet/readme/README.html

I wonder if this is a CRAN related issue or if the vignette was generated locally. sessionInfo would seem to suggest that maybe the vignette was generated on your computer @antagomir ?

sessionInfo()
#> R version 4.3.0 (2023-04-21)
#> Platform: x86_64-pc-linux-gnu (64-bit)
#> Running under: Ubuntu 22.04.3 LTS
antagomir commented 10 months ago

Yes I think the creation of the html vignette is part of the build procedure that creates the package tarball. The above mentioned vignette html file is there. Shall we try to fix these issues locally, then resubmit?

pitkant commented 10 months ago

I guess that would be one option, but if CRAN maintainers don't react to it then I wonder if it's appropriate to resubmit so early. From CRAN policies:

  • Once uploaded, no further submissions of that package should be made whilst the uploaded version is pending processing (which may take a few days) and you have not received a reply from a CRAN maintainer.
  • Submitting updates should be done responsibly and with respect for the volunteers’ time. Once a package is established (which may take several rounds), “no more than every 1–2 months” seems appropriate.

Maybe we can just note this and fix it for the next version

antagomir commented 10 months ago

Sure but it would nevertheless be good to troubleshoot this now so it is ready for the next.

Does the html render correctly for you, is this only my local issue?

pitkant commented 10 months ago

Yes, it does render correctly for me. Here's a screenshot of my locally rendered version:

sorvi_tutorial
Session info ``` sessionInfo() #> R version 4.3.1 (2023-06-16) #> Platform: aarch64-apple-darwin20 (64-bit) #> Running under: macOS Ventura 13.5 #> #> Matrix products: default #> BLAS: /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/lib/libRblas.0.dylib #> LAPACK: /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/lib/libRlapack.dylib; LAPACK version 3.11.0 #> #> locale: #> [1] C/fi_FI.UTF-8/fi_FI.UTF-8/C/fi_FI.UTF-8/fi_FI.UTF-8 #> #> time zone: Europe/Helsinki #> tzcode source: internal #> #> attached base packages: #> [1] stats graphics grDevices utils datasets methods base #> #> other attached packages: #> [1] ggplot2_3.4.3 sorvi_0.8.21 #> #> loaded via a namespace (and not attached): #> [1] tidyr_1.3.0 sass_0.4.7 utf8_1.2.3 generics_0.1.3 #> [5] class_7.3-22 xml2_1.3.5 KernSmooth_2.23-22 digest_0.6.33 #> [9] magrittr_2.0.3 evaluate_0.21 grid_4.3.1 timechange_0.2.0 #> [13] RColorBrewer_1.1-3 fastmap_1.1.1 jsonlite_1.8.7 e1071_1.7-13 #> [17] backports_1.4.1 DBI_1.1.3 httr_1.4.6 rvest_1.0.3 #> [21] purrr_1.0.2 fansi_1.0.4 scales_1.2.1 jquerylib_0.1.4 #> [25] cli_3.6.1 rlang_1.1.1 units_0.8-3 munsell_0.5.0 #> [29] withr_2.5.0 cachem_1.0.8 yaml_2.3.7 tools_4.3.1 #> [33] checkmate_2.2.0 dplyr_1.1.2 colorspace_2.1-0 vctrs_0.6.3 #> [37] R6_2.5.1 proxy_0.4-27 lifecycle_1.0.3 lubridate_1.9.2 #> [41] classInt_0.4-9 pkgconfig_2.0.3 pillar_1.9.0 bslib_0.5.1 #> [45] gtable_0.3.3 glue_1.6.2 gh_1.4.0 Rcpp_1.0.11 #> [49] sf_1.0-14 highr_0.10 xfun_0.40 tibble_3.2.1 #> [53] tidyselect_1.2.0 knitr_1.43 farver_2.1.1 htmltools_0.5.6 #> [57] rmarkdown_2.24 dlstats_0.1.7 compiler_4.3.1 ```
pitkant commented 8 months ago

With the minor bug fixes I just did the package should be ready for a new CRAN release

antagomir commented 8 months ago

Great.

I still wonder if geofi would be better home for the Finnish geo stuff. @muuankarski should we have a look together?

pitkant commented 8 months ago

Undoubtedly. Although I still see these historical mapping functionalities as somewhat experimental / WIP.