divy-kangeyan commented 6 years ago

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

Repository: https://github.com/aryeelab/scmeth

Confirm the following by editing each check box to '[x]'

[x] I understand that by submitting my package to Bioconductor, the package source and all review commentary are visible to the general public.
[x] I have read the Bioconductor Package Submission instructions. My package is consistent with the Bioconductor Package Guidelines.
[x] I understand that a minimum requirement for package acceptance is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS. Passing these checks does not result in automatic acceptance. The package will then undergo a formal review and recommendations for acceptance regarding other Bioconductor standards will be addressed.
[x] My package addresses statistical or bioinformatic issues related to the analysis and comprehension of high throughput genomic data.
[x] I am committed to the long-term maintenance of my package. This includes monitoring the support site for issues that users may have, subscribing to the bioc-devel mailing list to stay aware of developments in the Bioconductor community, responding promptly to requests for updates from the Core team in response to changes in R or underlying software.

I am familiar with the essential aspects of Bioconductor software management, including:

[x] The 'devel' branch for new packages and features.
[x] The stable 'release' branch, made available every six months, for bug fixes.
[x] Bioconductor version control using Git (optionally via GitHub).

For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

bioc-issue-bot commented 6 years ago

Hi @Divyagash

Thanks for submitting your package. We are taking a quick look at it and you will hear back from us soon.

The DESCRIPTION file for this package is:

Package: scmeth
Type: Package
Title: Functions to conduct quality control analysis in methylation data
Version: 0.99.0
Author: Divy Kangeyan <divyswar01@g.harvard.edu>
Depends: R (>= 3.4.0)
Imports:
  ggplot2,
  knitr,
  rmarkdown,
  bsseq,
  AnnotationHub,
  Repitools,
  GenomicRanges,
  reshape2,
  stats,
  utils,
  BSgenome,
  DelayedArray,
  annotatr,
  SummarizedExperiment,
  GenomeInfoDb,
  DT,
  ggthemes,
  scales,
  viridis,
  magrittr,
  dplyr
Suggests:
  BSgenome.Mmusculus.UCSC.mm10,
  BSgenome.Hsapiens.NCBI.GRCh38,
  Biobase,
  BiocGenerics,
  Biostrings
Maintainer: Divy Kangeyan <divyswar01@g.harvard.edu>
Description: Functions to analyze methylation data can be found here.
   Some functions are relevant for single cell methylation data but most other
   functions can be used for any  methylation data.
   Highlight of this workflow is the comprehensive quality control report.
biocViews: DNAMethylation, QualityControl, Preprocessing, SingleCell
License: GPL-2
LazyData: TRUE
RoxygenNote: 6.0.1
VignetteBuilder: knitr

Consider adding SSH keys to your GitHub account. SSH keys will are used to control access to accepted Bioconductor packages. See these instructions to add SSH keys to your GitHub account.

bioc-issue-bot commented 6 years ago

Your package has been approved for building. Your package is now submitted to our queue.

IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your repository will NOT trigger a new build.

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

divy-kangeyan commented 6 years ago

Not quite sure why there is this error ERROR: dependency Repitools is not available for package scmeth Build with travis seems to work fine, is this a problem with the build system?

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

Liubuntu commented 6 years ago

The ERROR you mentioned might be due to the building machine. Our people are working on that.

Please look at the new building report for "malbec2", there is another error in your vignette:

Quitting from lines 131-134 (my-vignette.Rmd) 
Error: processing vignette 'my-vignette.Rmd' failed with diagnostics:
'file' is not a slot in class "HDF5ArraySeed"
Execution halted

Please look to fix this while our people fixing the building error.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

26a9e24 Bumped up the version

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

205a477 Changed the directory of bs object in vignette

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

f1e1096 Moved the data files related to vignette to vignet...

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

divy-kangeyan commented 6 years ago

I am not getting any errors with the vignette when I use devtools::check in my local machine or in Travis CI. But the build in bioconductor is failing repeatedly. Any idea why this would happen?

Liubuntu commented 6 years ago

Hi, first please depend your package on R(>=3.5.0) in DESCRIPTION. Because the new packages will be in the Bioc devel 3.7 (not release 3.6), and the devel 3.7 will be consistent with the R development version of 3.5.

2nd, please check the package version for DelayedArray and SummarizedExperiment. These packages are under active development. Please make sure to use the most recent versions from github, which are 0.5.15 and 1.9.5.

Please do the above and regenerate files in your exdata folder, like this: directory<-system.file("extdata","bismark_data",package='scmeth') and manually run your vignette and let me know further message.

I suspect there is a little bug in the package of SummarizedExperiment but not quite sure yet. So please let me updated for the latest error. Thanks.

Qian

Liubuntu commented 6 years ago

I just made a pull request here: https://github.com/Bioconductor/SummarizedExperiment/pull/3

Please wait until it got merged into the master branch of SummarizedExperiment and use the updated version of 1.5.6, and regenerate your files in extdata. Let me know for furtuer questions.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

5cc034e Updated the version

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

e869b21 Updated the version of the SummarizedExperiment pa...

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

divy-kangeyan commented 6 years ago

@Liubuntu Thank you for the input. I made the version changes and I am getting the following error 'loadHDF5SummarizedExperiment' is not an exported object from 'namespace:SummarizedExperiment'

PeteHaitch commented 6 years ago

@Divyagash loadHDF5SummarizedExperiment() recently moved from SummarizedExperiment to HDF5Array (https://github.com/Bioconductor/SummarizedExperiment/commit/4faf8e1eca1e96a22da74e61cba4975db7d6bc40 and https://github.com/Bioconductor/HDF5Array/commit/21a0d4420069fc3618a4dd79070136c8952b41d2). You'll need to add HDF5Array to the 'Imports' of your DESCRIPTION file and import loadHDF5SummarizedExperiment() into your NAMESPACE

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

2ae1da2 Changed the loadSummarizedExperiment function to b...

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

191df5b Updated the loadHDF5SummarizedExperiment in vignet...

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

860d426 Imported loadHDF5SummarizedExperiment from HDF5Arr...

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "

Error response

Error code 414.

Message: Request-URI Too Long.

Error code explanation: 414 = URI is too long.. ". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself. Please see the [build report][1] for more details. [1]: http://bioconductor.org/spb_reports/scmeth_buildreport_20180115144514.html

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

92ab26a Check the build

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

15b6850 Change the header in one of the chunks

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

divy-kangeyan commented 6 years ago

I have made the specified version changes and I am currently using the function loadHDF5SummarizedExperiment from HDF5Array and I am having this previous error that I had no slot of name "filepath" for this object of class "HDF5ArraySeed"

Liubuntu commented 6 years ago

Hi, please depend on the newest version of HDF5Array package from github: https://github.com/Bioconductor/HDF5Array

The maintainer of HDF5Array has just moved the 2 functions of save/loadHDF5SummarizedExperiment from SummarizedExperiment into HDF5Array, and a test run of the these 2 functions seems work quite good for me.

Liubuntu commented 6 years ago

Did you import HDF5Array::loadHDF5SummarizedExperiment() into your NAMESPACE?

Liubuntu commented 6 years ago

Just a kind reminder that Bioconductor core team has a non-official rule for responses within 2 weeks of the technical review. Please make effort in addressing the issues and bump the version to trigger a valid build. But it does not necessarily to be fixing all within the 2 weeks. As long as you are making efforts in revising, the issue will keep open. Thanks!

Your error seems not a hard one. if you are using roxygen2 for generating the documentations, just add@importFrom HDF5Array loadHDF5SummarizedExperimen to have this function available in NAMESPACE, it will be ready to use.

Have a good weekend!

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

c17a43a Updated the version ofthe HDF5Array package

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

1a43053 Changed the loadHDF5SummarizedExperiment to be fro...

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

divy-kangeyan commented 6 years ago

HI @Liubuntu, Thank you for the input. Currently my NAMESPACE file looks like this

# Generated by roxygen2: do not edit by hand

export(bsConversionPlot)
export(chromosomeCoverage)
export(coverage)
export(cpgDensity)
export(cpgDiscretization)
export(downsample)
export(featureCoverage)
export(mbiasplot)
export(methylationDist)
export(readmetrics)
export(repMask)
export(report)
import(BSgenome)
import(DT)
import(GenomicRanges)
import(SummarizedExperiment)
import(knitr)
import(magrittr)
importFrom(DelayedArray,colSums)
importFrom(DelayedArray,rowSums)
importFrom(GenomeInfoDb,seqlevelsStyle)
importFrom(HDF5Array,loadHDF5SummarizedExperiment)
importFrom(annotatr,annotate_regions)
importFrom(annotatr,build_annotations)
importFrom(annotatr,builtin_genomes)
importFrom(annotatr,summarize_annotations)
importFrom(bsseq,getCoverage)
importFrom(ggthemes,theme_tufte)
importFrom(scales,comma)
importFrom(stats,dbinom)
importFrom(stats,relevel)
importFrom(stats,sd)
importFrom(utils,read.csv)
importFrom(utils,read.delim)
importFrom(utils,read.table)
importFrom(viridis,scale_fill_viridis)

And following is the content of the DESCRIPTION file

Package: scmeth
Type: Package
Title: Functions to conduct quality control analysis in methylation data
Version: 0.99.12
Author: Divy Kangeyan <divyswar01@g.harvard.edu>
Depends: R (>= 3.5.0)
Imports:
  ggplot2,
  knitr,
  rmarkdown,
  bsseq,
  AnnotationHub,
  Repitools,
  GenomicRanges,
  reshape2,
  stats,
  utils,
  BSgenome,
  DelayedArray (>= 0.5.15),
  annotatr,
  SummarizedExperiment (>= 1.5.6),
  GenomeInfoDb,
  DT,
  ggthemes,
  scales,
  viridis,
  magrittr,
  dplyr,
  HDF5Array (>= 1.7.5)
Suggests:
  BSgenome.Mmusculus.UCSC.mm10,
  BSgenome.Hsapiens.NCBI.GRCh38,
  Biobase,
  BiocGenerics,
  Biostrings
Maintainer: Divy Kangeyan <divyswar01@g.harvard.edu>
Description: Functions to analyze methylation data can be found here.
             Some functions are relevant for single cell methylation data but most other
             functions can be used for any  methylation data.
             Highlight of this workflow is the comprehensive quality control report.
biocViews: DNAMethylation, QualityControl, Preprocessing, SingleCell
License: GPL-2
LazyData: TRUE
RoxygenNote: 6.0.1
VignetteBuilder: knit

But this error no slot of name "filepath" for this object of class "HDF5ArraySeed" seems to be persistent. Is this due to some difference in HDF5ArraySeed class vs. HDF5Arrayclass?

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

93f98a8 Updated the new version of R

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

divy-kangeyan commented 6 years ago

I figured it is that the HDF5 files that I generated few weeks ago with previous version of the package are no longer compatible and I am working on this.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

6fd96e9 Updated the extdata files to be compatible with ne...

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

27f9ea9 substituted 1: to seq_len

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

Liubuntu commented 6 years ago

Hi,

your current error ERROR: dependency Repitools is not available for package scmeth is because that Repitools depends on an CRAN package which is not available. So nothing we can do now for this error. Not anyone's fault.

I will go ahead review your package in a few days.

Best, Qian

Liubuntu commented 6 years ago

Hi,

Thank you for submitting you package. Please look at the following issues and seek to address them. Always make a version bump to trigger a valid build. Please comment back when you are ready for a 2nd review. Thanks!

DESCRIPTION

Add a field for Maintainer. E.g.,

Author: Hervé Pagès
Maintainer: Hervé Pagès <hpages@fredhutch.org>

NAMESPACE

Looks good.

MAN/

add documentation for the scmeth package. with \type{package} and NULL in the body. Refer to: http://r-pkgs.had.co.nz/man.html#man-packages. Put this documentation in a file called
.R under R/. E.g.,
```
#' scmeth: a package to ....
#'
#' some description...
#' @docType package
#' @name scmeth
```

NULL

- similar and non-clear description for `repMask` and `cpgDensity`.
-  typo in `downsample.Rd` documentation. `\description{ Downsample the CpG covergae matrix for saturation analysis}`

### Vignette/
- Keep only the `my-vignette.Rmd` in this folder. Remove all unnecessary folders and files. The `.html` file and `.R` file will be generated automatically when installing the package. If any files are needed inside the vignette, you could put them under `inst/ext/` and use `system.file()`.
- put your `2017-04-21_HG23KBCXY_2_AGGCAGAA_TATCTC_pe.M-bias.txt` in `/inst/ext` and use `system.file()` to call it in vignette. Also modify the calling in example of `mbiasPlot.Rd`.
- add `sessionInfo()` in the end of the vignette.

### R/
- general: put white space before and after the assignment signal `<-` for assignment. Go through your script and revise all!
- If there is default value for arguments, you should specify them in the function documentation. e.g., `cpgDiscretization<-function(bs,subSample=1e6,offset=50000,coverageVec=NULL)`.

- `report.R`: Since the arguments of `organism` and `genome` only takes specific values, it's suggested to write them in this way:

report <- function(bsObj,outdirectory,organism=c("Mmusculus", "Hsapiens"), genome=c("mm10", "hg38"), ...) organism <- match.arg(organism) genome <- match.arg(genome)

Same for the R scripts of `cpgDensity`, `repMask` and `featureCoverage` which takes these kind of arguments. It will do the automatic check to avoid wrong input.

- `cpgDensity.R`: `cpgDensity<-function(bs,organism,windowLength=1000)`, the `organism` has been used in both `GenomeInfoDb::seqlevelsStyle(organism)` and `Repitools::cpgDensityCalc`. However, these 2 arguments has different specification for these 2 function. 1st requires that `must have a ‘seqlevels’ method or be a ‘character’ vector.` and the 2nd function requires that `organism: The‘BSgenome’ object to calculate CpG density upon.`. So there are some inconsistency in using of the same `organism` argument. And that's why your example returns an error:

cpgDensity(bs,Hsapiens,1000) Error in GenomeInfoDb::seqlevelsStyle(organism) : object 'Hsapiens' not found


Please check and revise your script.

### report: 

`inst/qcReport.Rmd`:L136, should the argument inside the function be param$genome instaed of genome? `featureCoverage(bs,c('genes_promoters','genes_exons',...), genome)`

divy-kangeyan commented 6 years ago

@Liubuntu Thank you for reviewing the package and I am working on those comments. Just a question on your third point in R/ for the report and other functions the organism variable is a BSgenome object so is it appropriate to use match.arg function and genome variable is a character but if I use match.arg doesn't that limit the genome builds that can be used in these functions?

Thank you!

Bioconductor / Contributions

scmeth #592

Error response

DESCRIPTION

NAMESPACE

MAN/