Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error running README examples #50

Open
cole-brokamp opened this issue Feb 2, 2024 · 5 comments
Open

error running README examples #50

cole-brokamp opened this issue Feb 2, 2024 · 5 comments

Comments

@cole-brokamp
Copy link

srtm <- terra::unwrap(readRDS("../../tests/testdata/nc_srtm15_otm.rds")) fails for me because I think the code is intended to work only if run from the development version of the package.

We could use a system.file() call like you did with the example sf data to get that to work (although tests are not installed by default if using the “testthat” package for tests; you might have to include this example in the package itself)

Using a README.Rmd to knit a README file would be a good way to include these in the CI process.

@sigmafelix
Copy link
Collaborator

Thank you for reporting the error. README.md is now based on README.Rmd to make sure that the code inside runs without errors. nc.shp from sf package is used in the example, but other data were reused from files in tests/testdata to reduce dependencies of the package. I rewrote the code to make the files be downloaded from the raw link of the file in chopin repository. Would it matter in submission to ROpenSci or CRAN? If it is the case, I will find another way to get a permanent link (e.g., in Zenodo or figshare) for example datasets.

Thank you for your help!

@cole-brokamp
Copy link
Author

You could put the example data in the /inst folder and then it would be installed with the package.

For example, the sf example data you are calling is located here: https://github.com/r-spatial/sf/tree/main/inst/shape and loaded with the command you have in the example: ncpoly <- system.file("shape/nc.shp", package = "sf")

Taking a similar approach for example data would prevent problems with downloading a file from the internet being required to run any examples.

I'd also suggest to remove the code about installing required packages for the readme example. If the README requires packages that are not installed with the package as dependencies, you can include them as suggested dependencies that will get installed. Or you could use rlang::is_installed() to check and tell user to install if not available. Cutting out a lot of this boilerplate code (especially if we are assuming users have a basic understanding of R) will likely increase the clarity of the README.

@sigmafelix
Copy link
Collaborator

The example data were not included in the package intentionally to reduce the file size of the built package (./tests/testdata folder is ~25 MB). Thank you for letting me know rlang::is_installed(). I found that rlang::check_installed() installs packages if there are any missing packages in users' system and loads them. Installation part in README.md gets dependencies = TRUE and its explanation for installing all required and suggested packages when chopin is installed.

I agree on making README succinct by removing boilerplate codes. Do you think the latest push includes boilerplate codes? Thank you!

@sigmafelix sigmafelix reopened this Feb 4, 2024
@cole-brokamp
Copy link
Author

Just my two cents, but dependencies = TRUE shouldn't be required because dependencies for an R package are automatically installed before the package is installed. To simplify even further, you could only suggest one way to install the package instead of three different ways.

You could host the data somewhere else if you don't want to include it in the package, but right now the README wouldn't be run-able without someone cloning the package to their computer. A workaround might be to host the data with the github release of the package as an asset.

@sigmafelix
Copy link
Collaborator

To make the README.md runnable, I will include the datasets used in README in inst/extdata as initially suggested. I think 25 MB of data might not be a big problem as the package size will still abide by 100 MB rules in CRAN. For installation scripts, I will keep remotes::install_github. Thank you for the comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants