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

change to appease cran #403

Closed
sckott opened this issue Nov 9, 2021 · 26 comments
Closed

change to appease cran #403

sckott opened this issue Nov 9, 2021 · 26 comments
Assignees

Comments

@sckott
Copy link
Contributor

sckott commented Nov 9, 2021


The CRAN policy only allows writing in file areas via
tools::R_user_dir() (which differ by OS). On macOS, ~/Library/Caches/R
is not one of those, yet we see there

drwxr-xr-x 5 ripley staff 160 8 Nov 23:35 ftdoi/
drwxr-xr-x 6 ripley staff 192 8 Nov 23:35 fulltext_storr/
drwxr-xr-x 2 ripley staff 64 8 Nov 06:54 noaa_ersst/
drwxr-xr-x 4 ripley staff 128 8 Nov 06:54 rdryad/

from

fulltext (also suppdata via fulltext)
ditto
rnoaa
mvSLOUCH via rdryad (which does not create this in its own checks).

respectively. NB: rappdirs does not comply with the policy.

This makes it much harder to sweep up after you.

These need to be corrected before 2021-11-23 to retain the package on CRAN.

djhocking added a commit that referenced this issue Nov 10, 2021
…ppease cran and fixed a few minor tests. Issue #403
@djhocking djhocking self-assigned this Nov 10, 2021
@djhocking
Copy link
Collaborator

I believe this is just a matter of switching out:

rappdirs::user_cache_dir("rnoaa")

with

tools::R_user_dir("rnoaa", which = "cache")

Fixed it with #0ba86af. Ran tests and build checks. Merging into master and will do final checks before releasing to cran (and updating version as appropriate).

@sckott
Copy link
Contributor Author

sckott commented Nov 13, 2021

👍🏽

@djhocking
Copy link
Collaborator

Success, update now on CRAN

@djhocking
Copy link
Collaborator

Unfortunately the fix apparently was not successful/sufficient. This is from Brian Ripley:

See the message below to the then maintainer of 'rnoaa'.
rnoaa has a new version and maintainer but still writes in the prohibited area. It is now scheduled for archival on Nov 30. That will necessitate archiving 'wildviz' which requires it, so please move rnoaa to Suggests and use it conditionally before that date.

@sckott - any ideas or suggestions on how to see if it’s writing to that folder on a Mac still? I don’t have one currently and am very new to docker.

@djhocking djhocking reopened this Nov 24, 2021
@sckott
Copy link
Contributor Author

sckott commented Nov 24, 2021

I would add skip_on_cran for this test block https://github.com/ropensci/rnoaa/blob/master/tests/testthat/test-ersst.R#L28-L36 - some of those test lines create the directory that he's so mad about - skipping those tests won't create it.

Just ran tests on my mac, i'd also skip_on_cran all the tests that create these folders:

  • noaa_arc2
  • noaa_cpc
  • noaa_ersst
  • noaa_ghcnd
  • noaa_isd
  • noaa_lcd
  • noaa_stormevents

Other things:

rappdirs is still used once in tests https://github.com/ropensci/rnoaa/blob/master/tests/testthat/test-ersst.R#L4 - so that should be removed, and rappdirs removed from imports - and this removed

the onload file https://github.com/ropensci/rnoaa/blob/master/R/onload.R has the creation of these cache folder objects - but nothing in that file creates anything on disk, only creates the R objects to handle throughout the pkg

@kjschaudt
Copy link

kjschaudt commented Nov 24, 2021 via email

@djhocking
Copy link
Collaborator

Thanks @sckott I completely missed that the rappdirs function was used above the individual tests in that script and was a bit panicked with the timeframe around the US holiday. I think it should be good now with rappdirs fully removed from the package in place of tools and those additional skip_on_cran test calls.

@kjschaudt - I think it's good to go. If you have time in the next day or two to build the package, run the tests, and see if anything gets written to ~/Library/Caches/R, it would definitely be appreciated. No worries if you don't have the time. I'll plan to do the cran release the evening of 28 November (tomorrow) or 29 November if you plan to try it but need another day.

Thanks to both of you

@kjschaudt
Copy link

kjschaudt commented Nov 27, 2021 via email

@djhocking
Copy link
Collaborator

Ken,

  1. I know v1.3.7 on CRAN writes to an unapproved location, so it's the current version on GitHub (1.3.8) that I want to make sure doesn't write to that folder.

  2. Yes, that is the correct test block, but if possible also running all the tests to see if anything ever gets written to ~/Library/Caches/R would be ideal. I use devtools::test() to run them all on the version cloned from GitHub.

We have the package built and tested on the current version of Mac OS via GitHub actions, so I am confident that all the tests work, but CRAN doesn't like that cache file location and I'm not sure how to be sure without running it on a Mac. As such, I think just running it on Catalina (or whichever is most convenient for you) would be sufficient. But again, it's not a big deal if it's more hassle than it's worth.

Thanks!

@kjschaudt
Copy link

kjschaudt commented Nov 28, 2021 via email

@kjschaudt
Copy link

kjschaudt commented Nov 28, 2021 via email

@kjschaudt
Copy link

kjschaudt commented Nov 28, 2021 via email

@djhocking
Copy link
Collaborator

Ken - thanks for your persistence with this!!! As a result, I think I might know what we need to change.

Can you run each of the following and let me know the results:

tools::R_user_dir("rnoaa")
tools::R_user_dir("rnoaa", which = "cache")

@kjschaudt
Copy link

kjschaudt commented Nov 28, 2021 via email

@djhocking
Copy link
Collaborator

Hmm, interesting. What about just these two lines outside of any test on Catalina?

tools::R_user_dir("rnoaa")
tools::R_user_dir("rnoaa", which = "cache")

@kjschaudt
Copy link

kjschaudt commented Nov 28, 2021 via email

@kjschaudt
Copy link

kjschaudt commented Nov 28, 2021 via email

@djhocking
Copy link
Collaborator

Each of those two lines can be run in the R console. Those are used internally in the package and are causing the problem. I'm thinking that the version without which = "cache" might solve the problem but I'll be curious to see the directory output of each.

git and GitHub are great but there's certainly a learning curve. Same with R. It's so unstructured relative to many other programming languages and the package developments are so fast that it's almost too flexible.

@kjschaudt
Copy link

kjschaudt commented Nov 28, 2021 via email

@djhocking
Copy link
Collaborator

djhocking commented Nov 28, 2021 via email

@kjschaudt
Copy link

kjschaudt commented Nov 28, 2021 via email

@kjschaudt
Copy link

kjschaudt commented Nov 28, 2021 via email

@kjschaudt
Copy link

kjschaudt commented Nov 28, 2021 via email

@djhocking
Copy link
Collaborator

Thanks for stepping through all this with me. It looks like this solution should work although I think it might require R >= 4.0 for tools >= 4.0 based on the output. I'm going to finish the CRAN checks and do the release. As such, I'm going to close this issue.

@kjschaudt wrt:

I was again unable to use testhat or devtools::test() to run your test code. WOULD BE PLEASED TO LEARN HOW TO DO THAT.

I think for devtools and testthat to really function as expected, you want to be using RStudio and be in a project and working directory of the package source code. To do this for rnoaa for example, you would

  1. Got to GitHub: https://github.com/ropensci/rnoaa and click Fork to create a copy of the full repository associated with your GitHub account.
  2. Clone that repository to your local machine. You can use the CL git clone function but the easiest option is to open RStudio and click Open New Project -> Check Out From Version Control -> GitHub -> the URL of your repository. This downloads all the files to your local machine and creates and R Project associated with it.
  3. At this point, you are ready for full development of the package and can run devtools::test() and all the other devtools functions on this package to create things and test them on your local machine.
    4.The R Package Git Book by Hadley Wickham and Jenny Bryan is the best source for starting developing and working with packages, including getting started with testing using the testthat package: https://r-pkgs.org/

@kjschaudt
Copy link

kjschaudt commented Nov 29, 2021 via email

@kjschaudt
Copy link

kjschaudt commented Nov 29, 2021 via email

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

3 participants