TransBioInfoLab / coMethDMR

Detect Regions of Concurrent Differential Methylation
https://transbioinfolab.github.io/coMethDMR/
7 stars 2 forks source link

NOTEs for Bioconductor Submission #1

Closed gabrielodom closed 2 years ago

gabrielodom commented 3 years ago

@fveitz, these are the notes we have before submission:

CRAN Checks

checking installed package size ... NOTE
    installed size is 14.2Mb
    sub-directories of 1Mb or more:
      data      1.3Mb
      doc       1.4Mb
      extdata  11.3Mb

checking for non-standard things in the check directory ... NOTE
  Found the following files/directories:
    ‘closeByRegions.rds’

Bioc Check

$error
character(0)

$warning
character(0)

$note
[1] "Recommended function length <= 50 lines."                                                                                                                                          
[2] "Usage of dontrun{} / donttest{} found in man page examples."                                                                                                                       
[3] "Consider shorter lines; 203 lines (5%) are > 80 characters long."                                                                                                                  
[4] "Consider multiples of 4 spaces for line indents, 986 lines(24%) are not."                                                                                                          
[5] "Cannot determine whether maintainer is subscribed to the bioc-devel\nmailing list (requires admin credentials).  Subscribe here:\nhttps://stat.ethz.ch/mailman/listinfo/bioc-devel"
gabrielodom commented 3 years ago

Updated NOTEs:

  1. 'LazyData:' in the 'DESCRIPTION' should be set to false or removed
  2. Update R version dependency from 4.0.0 to 4.1.
  3. Avoid the use of 'paste' in condition signalers. Found in files:
    • lmmTest.R (line 109, column 13)
    • lmmTestAllRegions.R (line 141, column 7; line 226, column 13; line 226, column 43)
    • WriteCloseByAllRegions.R (line 58, column 7; line 117, column 7)
  4. Avoid redundancy in signalers. Found in file lmmTestAllRegions.R (line 141, column 14)
  5. Recommended function length <= 50 lines.
  6. Consider shorter lines; 201 lines (5%) are > 80 characters long.
  7. Consider multiples of 4 spaces for line indents, 1002 lines(24%) are not.
gabrielodom commented 3 years ago

@fveitz, do you want to try to fix a few of these? No.3 is good. This NOTE means that we don't need a paste() call inside of a message() or warning() call. It's redundant.

Try out this example:

x <- "that"
message("try ", x, " out")
gabrielodom commented 3 years ago

@fveitz, here is a link explaining some more: https://bioconductor.org/packages/devel/bioc/vignettes/BiocCheck/inst/doc/BiocCheck.html#coding-practices-checks

mcanouil commented 3 years ago

2. Update R version dependency from 4.0.0 to 4.1.

Hi @gabrielodom is it really necessary to set such requirement? Are you using any of the new features of R 4.1 or relying on any of the really few breaking changes?

Edit: I am guessing it might be a Bioconductor guideline to enforce the R version this way ...

gabrielodom commented 3 years ago

Hi Dr. Canouil (@mcanouil), Bioconductor sets this requirement. Internally, I think our code is compatible with anything after v 3.1.0. Please let us know if you have any other questions. We are still trying to get this package published to Bioconductor (have been exceptionally busy teaching this summer).

mcanouil commented 3 years ago

Thanks, so I'll fork and make a change in requirement to avoid breaking my whole sets of analyses.

gabrielodom commented 3 years ago

Sounds great! Please keep us updated

gabrielodom commented 3 years ago

Link to Lori's new comments: https://github.com/Bioconductor/Contributions/issues/2064

gabrielodom commented 3 years ago

When re-building the package, make sure to update the cache location for ExperimentHub: https://bioconductor.org/packages/devel/bioc/vignettes/ExperimentHub/inst/doc/ExperimentHub.html#default-caching-location-update

gabrielodom commented 2 years ago

We are accepted!