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

Make gggenomes ready for CRAN submission #168

Merged
merged 49 commits into from
Mar 5, 2024
Merged

Make gggenomes ready for CRAN submission #168

merged 49 commits into from
Mar 5, 2024

Conversation

iimog
Copy link
Collaborator

@iimog iimog commented Oct 17, 2023

I'm trying to fix all the Errors, Warnings and Notes of R CMD check. I already fixed a couple. Still getting errors in the examples. Status is:

Status: 1 ERROR, 7 WARNINGs, 4 NOTEs

The 1 Error is misleading, because only fixing one turns up the next one 😅

I'll continue fixing stuff and add it to this pr. But all changes I made so far should be safe to merge anytime, to avoid conflicts in the future.

iimog added 8 commits October 17, 2023 21:27
"bold" is not a valid font family
this does not produce errors when I call it interactively
but R CMD check produces the "invalid font type" error.
Changing this to "sans" does not change the appearence of the plot for
me.
@thackl
Copy link
Owner

thackl commented Oct 17, 2023

Yeah, about what I expected. I already did some namespace stuff yesterday, but didn't follow through with all imported packages, in particular those that I use a lot. But if you want to find all functions from other packages, the code snippet from here works quite well: https://stackoverflow.com/questions/67492020/finding-objects-from-other-packages-namespaces-in-package-code

@thackl thackl added this to the v1.0 milestone Oct 17, 2023
@iimog
Copy link
Collaborator Author

iimog commented Oct 18, 2023

That looks really useful, indeed. Going 1-by-1 would have been really tedious.

@iimog
Copy link
Collaborator Author

iimog commented Oct 18, 2023

Update: all ERRORs are fixed. Now we have:

Status: 7 WARNINGs, 8 NOTEs

Again, I think the counts are underestimations :-D I will work on them, next.

@thackl
Copy link
Owner

thackl commented Oct 18, 2023

I'm guessing CRAN is anal about this and requires "WARNINGs/NOTEs" to be fixed?

@iimog
Copy link
Collaborator Author

iimog commented Oct 18, 2023

Yes, I think so, too.

@iimog
Copy link
Collaborator Author

iimog commented Oct 18, 2023

6 Notes are fixed. The warnings need a bit more attention. Just to be sure, to fix one of the notes I removed ggplot2 from Imports as we already have it in Depends. And I assume, that we want to keep ggplot2 in Depends, right?

@thackl
Copy link
Owner

thackl commented Oct 18, 2023

Yes, I think we should have ggplot2 in depends so people can always directly use any ggplot2 functions in the plot without having to load the library or use explicit ggplot2:: calls. Not even sure + stuff would work properly if we'd not depend on/library ggplot2 ;)

@iimog
Copy link
Collaborator Author

iimog commented Nov 30, 2023

I'll continue to fix warnings and notes, but there is one thing, that can not easily be fixed. The CRAN repository policy states, that:

Packages should be of the minimum necessary size. Reasonable compression should be used for data (not just .rda files) and PDF documentation: CRAN will if necessary pass the latter through qpdf.
As a general rule, neither data nor documentation should exceed 5MB (which covers several books). A CRAN package is not an appropriate way to distribute course notes, and authors will be asked to trim their documentation to a maximum of 5MB.
Where a large amount of data is required (even after compression), consideration should be given to a separate data-only package which can be updated only rarely (since older versions of packages are archived in perpetuity). [...]

Our check indicates, that:

* checking installed package size ... NOTE
  installed size is 100.4Mb
  sub-directories of 1Mb or more:
    doc       4.4Mb
    extdata  95.7Mb

So documentation is fine, but already close to the limit and extdata exceeds the limit. In order to fix this, we can consider a data-only package as suggested by the CRAN policy.

@iimog
Copy link
Collaborator Author

iimog commented Jan 19, 2024

I finally fixed all but one of the "no visible binding for global variable" notes. It was painful, I repeatedly broke stuff and readability of the code is not improved in my opinion. Anyway, it seems to be necessary for CRAN 😬
The remaining note about global variables is probably legit. It is about the function file_suffix here:

if(file_is_zip(file) && file_suffix(file, ignore_zip = FALSE) != "gz")

and again in line 23 of the same file. I was unable to find this function.
@thackl can you tell me from which package you are using this function? We probably need to add that as a dependency.

@iimog
Copy link
Collaborator Author

iimog commented Jan 19, 2024

When running R CMD check --as-cran I get additional NOTEs. I'll try to fix these as well. As cran our current status is:

Status: 8 NOTEs

@iimog
Copy link
Collaborator Author

iimog commented Jan 19, 2024

I'm finally at a point where almost everything is fixed 😅

For the remaining issues, I need some input from @thackl

  • update version number (and don't use "large components")
  • decrease package size (potentially by extracting the emales data to it's own data-only package)
  • stop using unexported objects via :::
  • fix usage of file_suffix (importing the package that provides it or re-implementing the functionality)
  • make examples run in less than 5s (maybe by skipping some or splitting them)
Full log
* using log directory ‘/home/markus/projects/gggenomes/gggenomes.Rcheck’
* using R version 4.3.2 (2023-10-31)
* using platform: x86_64-pc-linux-gnu (64-bit)
* R was compiled by
    gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
    GNU Fortran (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
* running under: Ubuntu 22.04.3 LTS
* using session charset: UTF-8
* using option ‘--as-cran’
* checking for file ‘gggenomes/DESCRIPTION’ ... OK
* this is package ‘gggenomes’ version ‘0.9.12.9000’
* package encoding: UTF-8
* checking CRAN incoming feasibility ... [6s/17s] NOTE
Maintainer: ‘Thomas Hackl ’

New submission

Version contains large components (0.9.12.9000)

Size of tarball: 48345619 bytes

  • 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 serialization versions ... OK
  • checking whether package ‘gggenomes’ can be installed ... OK
  • checking installed package size ... NOTE
    installed size is 100.3Mb
    sub-directories of 1Mb or more:
    doc 4.4Mb
    extdata 95.7Mb
  • 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 use of S3 registration ... OK
  • checking dependencies in R code ... NOTE
    Unexported objects imported by ':::' calls:
    ‘ellipsis:::dots’ ‘ggplot2:::make_labels’ ‘ggplot2:::scales_list’
    ‘scales:::force_all’
    See the note in ?::: about the use of this operator.
  • checking S3 generic/method consistency ... OK
  • checking replacement functions ... OK
  • checking foreign function calls ... OK
  • checking R code for possible problems ... NOTE
    read_gbk: no visible global function definition for ‘file_suffix’
    Undefined global functions or variables:
    file_suffix
  • checking Rd files ... OK
  • checking Rd metadata ... OK
  • checking Rd line widths ... OK
  • checking Rd cross-references ... OK
  • 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 LazyData ... OK
  • checking data for ASCII and uncompressed saves ... OK
  • checking sizes of PDF files under ‘inst/doc’ ... OK
  • checking installed files from ‘inst/doc’ ... OK
  • checking files in ‘vignettes’ ... OK
  • checking examples ... [63s/61s] NOTE
    Examples with CPU (user + system) or elapsed time > 5s
    user system elapsed
    flip 5.653 0.009 5.662
  • checking for unstated dependencies in ‘tests’ ... OK
  • checking tests ... OK
    Running ‘testthat.R’
  • checking for unstated dependencies in vignettes ...

@thackl
Copy link
Owner

thackl commented Mar 4, 2024

Wow, awesome job!!! Sorry, it took me soo long to pick it up again.

Re file_suffix #168 (comment) - I have now idea. I think this was actually at some point a function I added myself, but later deleted without noticing. I think we don't have a test that actually reads from gzipped genbank...

@thackl
Copy link
Owner

thackl commented Mar 4, 2024

I think I got most of it. Do you know which examples are too slow. Both splitting or skipping are fine with me.

@iimog
Copy link
Collaborator Author

iimog commented Mar 5, 2024

This is awesome 🚀
After a couple of package updates, the check went through with a single remaining NOTE (so the slow examples seem to be fast enough now 😄)

* checking installed package size ... NOTE
  installed size is  6.1Mb
  sub-directories of 1Mb or more:
    doc   4.4Mb

So I shrank the images in the emales vignette from 5400x2400 to 1620x720 which is still much larger than the display size in the html.

Now all the checks are passing locally.

* using log directory ‘/home/markus/projects/gggenomes/gggenomes.Rcheck’
* using R version 4.3.2 (2023-10-31)
* using platform: x86_64-pc-linux-gnu (64-bit)
* R was compiled by
    gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
    GNU Fortran (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
* running under: Ubuntu 22.04.4 LTS
* using session charset: UTF-8
* checking for file ‘gggenomes/DESCRIPTION’ ... OK
* this is package ‘gggenomes’ version ‘1.0.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 ‘gggenomes’ can be installed ... OK
* checking installed package size ... OK
* checking package directory ... 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 ... OK
* 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 cross-references ... OK
* 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 LazyData ... OK
* checking data for ASCII and uncompressed saves ... OK
* checking sizes of PDF files under ‘inst/doc’ ... OK
* checking installed files from ‘inst/doc’ ... OK
* checking files in ‘vignettes’ ... OK
* checking examples ... OK
* checking for unstated dependencies in ‘tests’ ... OK
* checking tests ... OK
  Running ‘testthat.R’
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in ‘inst/doc’ ... OK
* checking running R code from vignettes ... NONE
  ‘emales.Rmd’ using ‘UTF-8’... OK
  ‘gggenomes.Rmd’ using ‘UTF-8’... OK
* checking re-building of vignette outputs ... OK
* checking PDF version of manual ... OK
* DONE
Status: OK

So I think we are ready for CRAN submission 😎

@thackl thackl marked this pull request as ready for review March 5, 2024 15:14
@thackl thackl merged commit 88f81c6 into master Mar 5, 2024
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