luukvdmeer / sfnetworks

Tidy Geospatial Networks in R
https://luukvdmeer.github.io/sfnetworks/
Other
347 stars 20 forks source link

First draft of as.linnet.sfnetwork #101

Closed agila5 closed 3 years ago

agila5 commented 3 years ago

Refs https://github.com/spatstat/spatstat/issues/145 and https://github.com/luukvdmeer/sfnetworks/issues/41#issuecomment-745259778.

As I said in the other github issue, I'm not sure about the best approach for this PR, i.e. I'm not sure about the roxygen docs and the naming conventions. It works (and, IMO, it's useful code) but clearly feel free to add any comment. Moreover, if you plan to define other methods to convert sfnetworks object into other classes, I would create an ad-hoc file instead of appending everything into sfnetworks.R.

agila5 commented 3 years ago

I have no idea why the R CMD check fails since I see no error when I run devtools::check(args = c('--no-manual','--as-cran')) on my laptop πŸ˜…

luukvdmeer commented 3 years ago

I think the problem here is that linnet is a class defined in the spatstat package, which is not a dependency of sfnetworks. Creating a method for a class from a non-dependency package is a bit more complicated than for dependency packages. We had the same issue with creating a method for ggplot2::autoplot. @loreabad6 knows better how to handle this.

Regarding the file structure. I think we should leave sfnetwork.R only for creating sfnetworks. So for sfnetwork() and all as_sfnetwork() methods. I think this specific function can be in a separate linnet.R file, just as as_tibble.sfnetwork is in a separate tibble.R file.

loreabad6 commented 3 years ago

Hi @agila5, what we did for ggplot2::autoplot() is registering an S3 method on load, so only if the person has spatstat, this method is registered otherwise not. It is now in the plot.R file, but if we have more registration tasks then I would create a zzz.R file with this. I will do these changes on your PR and push to see if it passes the checks πŸ˜„

loreabad6 commented 3 years ago

Seemed to have worked! So I leave up to you @agila5 the reorganization of the files...

Regarding the file structure. I think we should leave sfnetwork.R only for creating sfnetworks. So for sfnetwork() and all as_sfnetwork() methods. I think this specific function can be in a separate linnet.R file, just as as_tibble.sfnetwork is in a separate tibble.R file.

... and then any other changes to then merge to develop! Thank you! πŸ˜„

agila5 commented 3 years ago

Wow, thank you very much. So many things that I still need to learn and not enough time 😱 I will complete the PR after lunch. Again, thanks!

agila5 commented 3 years ago

I think it's ready to be merged.

luukvdmeer commented 3 years ago

Thanks for your work @agila5 @loreabad6! I'll merge this tomorrow!

Robinlovelace commented 3 years ago

Heads-up on this @agila5 I received this email as maintainer of a package that uses spatstat:

Dear package maintainer:

                       -- Imminent changes to spatstat package --

                       -- details have changed ---

You are listed as the maintainer of a CRAN package which depends on the package 'spatstat'.

We want to inform you that 'spatstat' is about to be divided into 7 sub-packages (as mandated by CRAN). The sub-packages already exist on GitHub for you to try.

This is advance notice of the change. If you choose to do nothing about it, everything will be OK until about 06 February 2021 when you will probably receive an email from CRAN saying that your package does not pass the package checker any more, and asking you to update it.

THE NEW STRUCTURE

In the new structure, when you install 'spatstat', the following packages will be installed:

                 spatstat.utils (low level utilities)

                 spatstat.data (datasets)

                 spatstat.sparse (sparse 3D arrays and linear algebra)

                 spatstat.geom (classes of geometrical objects;

                                            geometrical operations)

                 spatstat.core (statistical calculations) 

                 spatstat.linnet (code for linear networks)  

                 spatstat (umbrella package containing only documentation)

Each package in this list requires all the previous packages in the list.

The code will remain effectively unchanged; it will just be redistributed into different packages. For R users, typing library(spatstat) will still work as it does now.

WHAT THIS MEANS FOR YOUR PACKAGE

All existing functions and data in spatstat will continue to be available in the new structure when library(spatstat) is loaded.

The main implication for your package is that cross-references to the help files will have to be edited, and you may need to adjust the DESCRIPTION and NAMESPACE files.

CURRENT STATUS

The new packages are all ready to submit to CRAN. You can find them at the following locations on GitHub.

PACKAGE GITHUB LOCATION spatstat.utils spatstat/spatstat.utils (ref=master) spatstat.data spatstat/spatstat.data (ref=master) spatstat.sparse spatstat/spatstat.sparse (ref=master) spatstat.geom spatstat/spatstat.geom (ref=main) spatstat.core spatstat/spatstat.core (ref=master) spatstat.linnet baddstats/spatstat.linnet (ref=master) spatstat (umbrella package) baddstats/spatstat (ref=main) **SEE NOTE BELOW

Note that the umbrella 'spatstat' package is at baddstats/spatstat, whereas the existing, 'big old' spatstat is at spatstat/spatstat.

To avoid grief, we suggest you

un-install any existing version of spatstat.core
update/install the package 'remotes'
install the current versions of the spatstat sub-packages from GitHub in the order listed above, from top to bottom, using the correct 'ref' listed above.

WHAT WILL HAPPEN NEXT

between now and 04 February 2021:

We suggest that you start privately preparing a new version of your package that uses the umbrella​ spatstat package. Don't submit this package until 06 February 2021 or later.

on 4 January 2021: when CRAN reopens for submissions, we will start submitting the sub-packages. Each one requires a few days to percolate through CRAN before we can submit the next one.

about 15-20 January 2021: You will receive an email notification that all the sub-packages are available on CRAN (except for the umbrella package 'spatstat').

about 04 february 2021: we will submit the umbrella spatstat package to CRAN.

Shortly after this, you may receive an email from CRAN saying that your package does not pass the package checker with the new spatstat. We suggest that you wait for this email from CRAN, fix any additional issues that have been detected, and submit the new version of your package.

We are sorry for the inconvenience that this will cause.

We would be happy to assist you with advice for this transition

agila5 commented 3 years ago

Heads-up on this @agila5 I received this email as maintainer of a package that uses spatstat:

Hi @Robinlovelace. This is related to stats19, right? If so I will open a new issue there and test the package with the new structure of spatstat. The relations between stats19 and spatstat are really minimal so I think it won't be a big deal. I will also test the same things here in sfnetworks repo.

Robinlovelace commented 3 years ago

I followed the instructions above and tested the new branch as follows:

 remotes::install_github("baddstats/spatstat")
Using github PAT from envvar GITHUB_PAT
Downloading GitHub repo baddstats/spatstat@HEAD
βœ”  checking for file β€˜/tmp/RtmpV8Au0e/remotes1af2759a96826/baddstats-spatstat-4f7a395/DESCRIPTION’ ...
─  preparing β€˜spatstat’:
βœ”  checking DESCRIPTION meta-information ...
─  checking for LF line-endings in source and make files and shell scripts
─  checking for empty or unneeded directories
─  building β€˜spatstat_2.0-0.tar.gz’

Installing package into β€˜/usr/local/lib/R/site-library’
(as β€˜lib’ is unspecified)
* installing *source* package β€˜spatstat’ ...
** using staged installation
** R
** demo
** inst
** byte-compile and prepare package for lazy loading
** help
*** installing help indices
** building package indices
** installing vignettes
   β€˜bugfixes.Rnw’ 
   β€˜datasets.Rnw’ 
   β€˜getstart.Rnw’ 
   β€˜replicated.Rnw’ 
   β€˜shapefiles.Rnw’ 
   β€˜updates.Rnw’ 
** testing if installed package can be loaded from temporary location
** testing if installed package can be loaded from final location
** testing if installed package keeps a record of temporary installation path
* DONE (spatstat)
> devtools::check()
Updating sfnetworks documentation
Loading sfnetworks
Writing NAMESPACE
Writing NAMESPACE
── Building ────────────────────────────────────────────────────────────────────────────────────────────────────────────────── sfnetworks ──
Setting env vars:
● CFLAGS    : -Wall -pedantic -fdiagnostics-color=always
● CXXFLAGS  : -Wall -pedantic -fdiagnostics-color=always
● CXX11FLAGS: -Wall -pedantic -fdiagnostics-color=always
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
βœ”  checking for file β€˜/home/robin/other-repos/sfnetworks/DESCRIPTION’ ...
─  preparing β€˜sfnetworks’:
βœ”  checking DESCRIPTION meta-information ...
─  installing the package to build vignettes
βœ”  creating vignettes (29.1s)
─  checking for LF line-endings in source and make files and shell scripts
─  checking for empty or unneeded directories
─  building β€˜sfnetworks_0.4.0.tar.gz’

── Checking ────────────────────────────────────────────────────────────────────────────────────────────────────────────────── sfnetworks ──
Setting env vars:
● _R_CHECK_CRAN_INCOMING_USE_ASPELL_: TRUE
● _R_CHECK_CRAN_INCOMING_REMOTE_    : FALSE
● _R_CHECK_CRAN_INCOMING_           : FALSE
● _R_CHECK_FORCE_SUGGESTS_          : FALSE
● NOT_CRAN                          : true
── R CMD check ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
* using log directory β€˜/tmp/RtmpV8Au0e/sfnetworks.Rcheck’
* using R version 4.0.3 (2020-10-10)
* using platform: x86_64-pc-linux-gnu (64-bit)
* using session charset: UTF-8
* using options β€˜--no-manual --as-cran’
* checking for file β€˜sfnetworks/DESCRIPTION’ ... OK
* this is package β€˜sfnetworks’ version β€˜0.4.0’
* package encoding: UTF-8
* checking package namespace information ... OK
* checking package dependencies ... OK
* checking if this is a source package ... OK
* checking if there is a namespace ... OK
* checking for executable files ... OK
* checking for hidden files and directories ... OK
* checking for portable file names ... OK
* checking for sufficient/correct file permissions ... OK
* checking whether package β€˜sfnetworks’ can be installed ... OK
* checking installed package size ... OK
* checking package directory ... OK
* checking for future file timestamps ... OK
* checking β€˜build’ directory ... OK
* checking DESCRIPTION meta-information ... OK
* checking top-level files ... OK
* checking for left-over files ... OK
* checking index information ... OK
* checking package subdirectories ... OK
* checking R files for non-ASCII characters ... OK
* checking R files for syntax errors ... OK
* checking whether the package can be loaded ... OK
* checking whether the package can be loaded with stated dependencies ... OK
* checking whether the package can be unloaded cleanly ... OK
* checking whether the namespace can be loaded with stated dependencies ... OK
* checking whether the namespace can be unloaded cleanly ... OK
* checking loading without being on the library search path ... OK
* checking dependencies in R code ... NOTE
Missing or unexported objects:
  β€˜spatstat::as.ppp’ β€˜spatstat::as.psp’ β€˜spatstat::linnet’
* checking S3 generic/method consistency ... OK
* checking replacement functions ... OK
* checking foreign function calls ... OK
* checking R code for possible problems ... OK
* checking Rd files ... OK
* checking Rd metadata ... OK
* checking Rd line widths ... OK
* checking Rd cross-references ... WARNING
Missing link or links in documentation object 'as.linnet.Rd':
  β€˜[spatstat]{linnet}’

See section 'Cross-references' in the 'Writing R Extensions' manual.

* checking for missing documentation entries ... OK
* checking for code/documentation mismatches ... OK
* checking Rd \usage sections ... OK
* checking Rd contents ... OK
* checking for unstated dependencies in examples ... OK
* checking contents of β€˜data’ directory ... OK
* checking data for non-ASCII characters ... OK
* checking data for ASCII and uncompressed saves ... OK
* checking installed files from β€˜inst/doc’ ... OK
* checking files in β€˜vignettes’ ... OK
* checking examples ... ERROR
Running examples in β€˜sfnetworks-Ex.R’ failed
The error most likely occurred in:

> base::assign(".ptime", proc.time(), pos = "CheckExEnv")
> ### Name: as.linnet
> ### Title: Convert an sfnetwork object into linnet format
> ### Aliases: as.linnet as.linnet.sfnetwork
> 
> ### ** Examples
> 
> if (require("spatstat", quietly = TRUE)) {
+   roxel_sfn = as_sfnetwork(roxel) %>%
+     sf::st_transform(3035)
+   (roxel_linnet = as.linnet(roxel_sfn, sparse = TRUE))
+   plot(roxel_linnet)
+ }
spatstat.geom 1.65-0
spatstat.core 1.65-0
spatstat.linnet 1.65-0

spatstat 1.65-0       (nickname: β€˜Famous Last Words’) 
For an introduction to spatstat, type β€˜beginner’ 

Error: 'as.ppp' is not an exported object from 'namespace:spatstat'
Execution halted
* checking for unstated dependencies in β€˜tests’ ... OK
* checking tests ...
  Running β€˜testthat.R’
 OK
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in β€˜inst/doc’ ... OK
* checking re-building of vignette outputs ... OK
* checking for non-standard things in the check directory ... OK
* checking for detritus in the temp directory ... OK
* DONE

Status: 1 ERROR, 1 WARNING, 1 NOTE
See
  β€˜/tmp/RtmpV8Au0e/sfnetworks.Rcheck/00check.log’
for details.

── R CMD check results ─────────────────────────────────────────────────────────────────────────────────────────────── sfnetworks 0.4.0 ────
Duration: 1m 15.4s

❯ checking examples ... ERROR
  Running examples in β€˜sfnetworks-Ex.R’ failed
  The error most likely occurred in:

  > base::assign(".ptime", proc.time(), pos = "CheckExEnv")
  > ### Name: as.linnet
  > ### Title: Convert an sfnetwork object into linnet format
  > ### Aliases: as.linnet as.linnet.sfnetwork
  > 
  > ### ** Examples
  > 
  > if (require("spatstat", quietly = TRUE)) {
  +   roxel_sfn = as_sfnetwork(roxel) %>%
  +     sf::st_transform(3035)
  +   (roxel_linnet = as.linnet(roxel_sfn, sparse = TRUE))
  +   plot(roxel_linnet)
  + }
  spatstat.geom 1.65-0
  spatstat.core 1.65-0
  spatstat.linnet 1.65-0

  spatstat 1.65-0       (nickname: β€˜Famous Last Words’) 
  For an introduction to spatstat, type β€˜beginner’ 

  Error: 'as.ppp' is not an exported object from 'namespace:spatstat'
  Execution halted

❯ checking Rd cross-references ... WARNING
  Missing link or links in documentation object 'as.linnet.Rd':
    β€˜[spatstat]{linnet}’

  See section 'Cross-references' in the 'Writing R Extensions' manual.

❯ checking dependencies in R code ... NOTE
  Missing or unexported objects:
    β€˜spatstat::as.ppp’ β€˜spatstat::as.psp’ β€˜spatstat::linnet’

1 error βœ– | 1 warning βœ– | 1 note βœ–
Robinlovelace commented 3 years ago

Before the tests I installed the latest version of the spatstat packages as follows:

remotes::install_github("spatstat/spatstat.utils")
#> Using github PAT from envvar GITHUB_PAT
#> Skipping install of 'spatstat.utils' from a github remote, the SHA1 (5f9820cf) has not changed since last install.
#>   Use `force = TRUE` to force installation
remotes::install_github("spatstat/spatstat.data")
#> Using github PAT from envvar GITHUB_PAT
#> Skipping install of 'spatstat.data' from a github remote, the SHA1 (4989f46d) has not changed since last install.
#>   Use `force = TRUE` to force installation
remotes::install_github("spatstat/spatstat.sparse")
#> Using github PAT from envvar GITHUB_PAT
#> Skipping install of 'spatstat.sparse' from a github remote, the SHA1 (e81696e2) has not changed since last install.
#>   Use `force = TRUE` to force installation
remotes::install_github("spatstat/spatstat.geom")
#> Using github PAT from envvar GITHUB_PAT
#> Skipping install of 'spatstat.geom' from a github remote, the SHA1 (4dfed8d1) has not changed since last install.
#>   Use `force = TRUE` to force installation
remotes::install_github("baddstats/spatstats.linnet")
#> Using github PAT from envvar GITHUB_PAT
#> Error: Failed to install 'unknown package' from GitHub:
#>   HTTP error 404.
#>   Not Found
#> 
#>   Did you spell the repo owner (`baddstats`) and repo name (`spatstats.linnet`) correctly?
#>   - If spelling is correct, check that you have the required permissions to access the repo.
remotes::install_github("baddstats/spatstat")
#> Using github PAT from envvar GITHUB_PAT
#> Skipping install of 'spatstat' from a github remote, the SHA1 (4f7a3952) has not changed since last install.
#>   Use `force = TRUE` to force installation

Created on 2020-12-23 by the reprex package (v0.3.0)

Robinlovelace commented 3 years ago

Hi @Robinlovelace. This is related to stats19, right?

Correct but it may have implications for this PR, hence communicating here first. Hopefully a small fix will resolve the issue on stats19, can check on my laptop which has the new spatstat packages.

agila5 commented 3 years ago

Ok, for the moment I think we can only wait and test everything again in 30 days.