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

CRAN prep #19

Merged
merged 18 commits into from
Nov 10, 2022
Merged

CRAN prep #19

merged 18 commits into from
Nov 10, 2022

Conversation

AustinHartman
Copy link
Contributor

@AustinHartman AustinHartman commented Nov 2, 2022

Work in progress preparing presto for CRAN submission. Looking for any feedback related to styling, documentations, etc.

I moved DESeq2 from imports to suggests given it's used for the pseudobulk testing, but not for the main wilcox rank sum workflow. Also, since DESeq2 is a bioconductor package it will make it easier for CRAN packages, like Seurat, to import presto.

@AustinHartman AustinHartman marked this pull request as draft November 2, 2022 20:17
@slowkow
Copy link
Member

slowkow commented Nov 3, 2022

Hi Austin, thanks for getting this started. At a glance, I like your first round of changes.

Our GitHub actions are failing:

Error: Unable to resolve action `r-lib/actions@master`, unable to find version `master`

Could I please ask you to go ahead and make these changes to the .github/workflows/R-CMD-check.yaml file:

- uses: r-lib/actions/setup-r@master

This should be:

r-lib/actions/setup-r@v2.3.0

- uses: r-lib/actions/setup-pandoc@master

This should be:

r-lib/actions/setup-pandoc@v2.3.0

I think the branch called master was removed from the r-lib/actions repo, so we should use the tags instead.

@AustinHartman
Copy link
Contributor Author

AustinHartman commented Nov 8, 2022

A few changes to the GHA:

  • use more recent ubuntu versions as I don't think v16 ubuntu runners are still available
  • only test on R 4.1 and devel. Multiple dependencies require R>=4 and one of DESeq2's dependencies requires >=4.1

I played around with the GHA on my fork of the repo and checks passed on windows and macOS but not yet on ubuntu which I'm still sorting out

@slowkow
Copy link
Member

slowkow commented Nov 9, 2022

Hey Austin,

Thanks for working on this. Below, I share some notes on what I could observe from the logs.

In summary, I think we're on the right track, and I believe many potential GHA failures are out of your control. I might suggest running R CMD check on your laptop to avoid losing time due to these GHA issues.

Thanks for your help and patience,
Kamil

Details

I noticed in your repo that macOS passed the GHA but the others did not. Weird.

I looked at one of the failed GHA runs and found this:

Quitting from lines 117-120 (getting-started.Rmd) 
Error: Error: processing vignette 'getting-started.Rmd' failed with diagnostics:
unable to find required package 'SingleCellExperiment'
--- failed re-building ‘getting-started.Rmd’
SUMMARY: processing the following file failed:
  ‘getting-started.Rmd’
Error: Error: Vignette re-building failed.
Execution halted

Here are the lines:

```{r}
data(object_sce)
head(wilcoxauc(object_sce, 'cell_type'))
```

At first I thought maybe we failed to add SingleCellExperiment to the dependencies. But I checked DESCRIPTION and see that we include the package in Suggests.

Then I checked the log file in GHA and found this:

trying URL 'https://bioconductor.org/packages/3.14/bioc/src/contrib/SingleCellExperiment_1.16.0.tar.gz'
Error in download.file(url, destfile, method, mode = "wb", ...) : 
  cannot open URL 'https://bioconductor.org/packages/3.14/bioc/src/contrib/SingleCellExperiment_1.16.0.tar.gz'
In addition: Warning message:
In download.file(url, destfile, method, mode = "wb", ...) :
  cannot open URL 'https://mghp.osn.xsede.org/bir190004-bucket01/archive.bioconductor.org/packages/3.14/bioc/src/contrib/SingleCellExperiment_1.16.0.tar.gz': HTTP status was '503 Service Unavailable'
Warning in download.packages(pkgs, destdir = tmpd, available = available,  :
  download of package ‘SingleCellExperiment’ failed

This indicates that the package is not downloading successfully. Maybe we are unlucky.

Just now, I was able to reproduce the 503 error in my web browser. A few moments later, I am now able to successfully download this file:

https://bioconductor.org/packages/3.14/bioc/src/contrib/SingleCellExperiment_1.16.0.tar.gz

I might conclude that the bioconductor server was momentarily unavailable.

Then, I decided to launch the GHA jobs in this pull request and I saw a new error message:

Run Rscript -e "remotes::install_github('r-hub/sysreqs')"
Using bundled GitHub PAT. Please add your own PAT to the env var `GITHUB_PAT`
Error: Error: Failed to install 'unknown package' from GitHub:
  cannot open URL 'https://api.github.com/repos/r-hub/sysreqs/contents/DESCRIPTION?ref=HEAD'
Execution halted
Error: Process completed with exit code 1.

According to this comment, the error might go away if we just re-run the GHA jobs:
r-lib/remotes#130 (comment)

Conclusions

  • Since your code passed GHA macOS devel, I think it is likely that it will pass on other platforms and versions.

  • Please consider using R CMD check on your laptop instead of relying on the output from GHA, since it can be slow and fail unpredictably at times. I have lost many hours to weird GHA errors. I wish the caching would work more reliably, so we could avoid network errors.

How to run R CMD check quickly

Recently, I had to run CRAN check on ggrepel a few times, and I was frustrated that each run took a lot of time. Then I found a comment by Jim Hester:

You can set the following environment variables to false to turn off the
incoming checks. The first turns off all incoming checks, the second only
turns off those checks which use remote resources (i.e. the internet). The
latter is only available on relatively new versions of R.
Jim Hester

#export _R_CHECK_CRAN_INCOMING_=false
export _R_CHECK_CRAN_INCOMING_REMOTE_=false

R CMD build ggrepel

R CMD check --as-cran ggrepel_0.9.2.tar.gz

After setting the environment variable _R_CHECK_CRAN_INCOMING_REMOTE_ my checks were running much more quickly, so my development cycle became much faster.

@AustinHartman
Copy link
Contributor Author

Thanks for investigating the logs and suggestions. I've been running R checks locally without any errors warnings or notes on an ubuntu 20.04 system with R 4.1.3, so the GHA errors were indeed perplexing. Since the GHA errors seem to be due to inconsistency in dependency installation, I can focus a bit more on documentation. Anything else I'm missing before merging and CRAN submission?

@slowkow slowkow marked this pull request as ready for review November 9, 2022 16:36
@slowkow
Copy link
Member

slowkow commented Nov 9, 2022

Thank you, Austin!

Would you like to make more changes (e.g. clearer error messages, documentation)?

One thing that users might appreciate is a pkgdown website with some usage examples and tips. I think an appropriate URL for this page would be https://immunogenomics.github.io/presto

If you're ready for a merge, please let me know and I'll take it from here.

@AustinHartman
Copy link
Contributor Author

I've made a few changes to the docs and examples and added a pkgdown site in the recent commits. Let me know if there are other changes you'd like, otherwise I think this is ready to merge

@slowkow slowkow merged commit 81f1305 into immunogenomics:master Nov 10, 2022
@slowkow
Copy link
Member

slowkow commented Nov 10, 2022

@AustinHartman I forgot to ask, could you please provide your information so we can add you as a contributor in the DESCRIPTION file?

Here's mine (I will add myself, too, since I'm contributing):

person("Kamil", "Slowikowski", role = c("ctb"), comment = c(ORCID = "0000-0002-2843-6370") )

@AustinHartman
Copy link
Contributor Author

AustinHartman commented Nov 10, 2022

Thanks - here's my info: person("Austin", "Hartman", role = c("ctb"), comment = c(ORCID = "0000-0001-7278-1852"))

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

Successfully merging this pull request may close these issues.

2 participants