Closed michaeldshapiro closed 5 years ago
Hi @michaeldshapiro
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: deltaCaptureC
Title: This Package Discovers Meso-scale Chromatin Remodeling from 3C Data
Version: 0.99.0
Authors@R:
person(given = "Michael",
family = "Shapiro",
role = c("aut", "cre"),
email = "sifka@earthlink.net",
comment = c(ORCID = "0000-0002-2769-9320"))
Description: This package discovers meso-scale chromatin remodelling from 3C data. 3C data is local in nature. It givens interaction counts between restriction enzyme digestion fragments and a preferred 'viewpoint' region. By binning this data and using permutation testing, this package can test whether there are statistically significant changes in the interaction counts between the data from two cell types or two treatments.
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
Depends: R (>= 3.5.0)
Imports:
IRanges,
GenomicRanges,
SummarizedExperiment,
ggplot2,
DESeq2
Roxygen: list(markdown = TRUE)
RoxygenNote: 6.1.1
Suggests:
knitr,
rmarkdown
VignetteBuilder: knitr
biocViews:
BiologicalQuestion,
StatisticalMethod
A reviewer has been assigned to your package. Learn what to expect during the review process.
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.
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". 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.
The ERRORs have now been rectified and the code has been modified to address suggestions.
I am new to this process, so I am not sure what to expect once the errors have been corrected. In particualr, I'm not sure what triggers a new check once the errors have been corrected and the code changes committed to git. Please advise if I need to undertake any additional steps.
Please see the link for adding a webhook that was also referenced above. This will trigger a new build when there is a version bump in the DESCRIPTION file. Once you have set up the webhook, bump your version, and see if the ERRORs remain on the corresponding build report.
Thank you, lshep. Just what the doctor ordered.
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: "TIMEOUT, WARNINGS". 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.
Received a valid push; starting a build. Commits are:
67d09e5 Added a smaller set of small bins and a smaller de...
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: "WARNINGS". 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.
Received a valid push; starting a build. Commits are:
a85a059 Bumped version number
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: "WARNINGS". 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.
The remaining warning, 'Evaluate more vignette chunks' seems rather generic. Maybe this package is ready to move on to the next stage in its evaluation?
Cheers!
Chipping in here; if you're dealing with genomic interaction data, you may want to supporting inputs of the GenomicInteractions
class. One example might be to accept a GenomicInteractions
object for the restriction fragment pairs annotated with counts. We have a whole stack of infrastructure based on this class for handling various types of chromosomal conformation data, see the revdeps here and here.
Thanks for alerting me to this. I will look into into it.
ok
ok
ok
ok
Import functions like DESeq2::estimateSizeFactorsForMatrix because you packge needs it. Use the @importFrom roxygen functionality.
Same with SummarizedExperiment. You should put all these functions in the NAMESPACE, and make your code more readable. By removing calls to "SummarizedExperiment::" in everyplace you use that function, you will make your code more readable.
Review:
http://bioconductor.org/developers/how-to/coding-style/
http://bioconductor.org/developers/package-guidelines/#namespace
You have done this with other packages as well. please correct it.
It seems like some loops can be avoided. Please keep in mind that division is vectorized in R. You can have column wise division.
For argument validation, use,
if(condition)
stop("<Explain why something has to be a value>")
vs
stopifnot(length(unique(W)) == 1)
In this loop, you will be calcualting the value of ceiling(n-m)/2
each time
for(i in seq_len(ceiling((n-m)/2)))
vs
val <- ceiling(n-m)/2
for (i in seq_len(val)) {
...
}
This is not efficient. Please correct similar ideas everywhere in your code base.
Move the code for function getRunsAndTotals
outside of
getRunTotals
. Make use of private functions, by not exporting them
to your namespace.
Define the function, .getRunsAndTotals()
outside
getRunTotals
. Look up coding style and standards on the
Bioconductor website for Developers.
Dear Nitesh -
Thank you for your comments. They seem very much to the point. I've changed jobs since the initial submission and am juggling obligations, but I hope to respond to these soon.
Thanks, Michael
@michaeldshapiro
Congrats on your new job. Just as a heads up. We usually allow a 14 day period for each issue to receive a response once a review is posted. But not to worry, if you do go beyond that, we can close the issue and you can just click "reopen" once you are ready for further development.
Best,
Nitesh
Received a valid push; starting a build. Commits are:
e07c2bb This represents an overhaul to respond to suggest...
Dear Nitesh -
Thank you for the heads-up on the deadline. Makes a great excuse for putting everything else aside.
I'm glad to know about the @importFrom technology. Makes for much happier code. Is there a way to rationalize the placement of the imports? I would have liked to place them all first in the file, but this leads to devtools::document() warnings.
I looked at my for loops, but did not have much success vectorizing these. Most of these iterate either over individual assays in a SummarizedExperiment or over the columns of a matrix. Sadly, although the call to width(intersect(...)) can be vectorized, intersect() does not report empty intersections which leads to omission of 0 widths, hence a vector of the wrong length.
Looking forward to your thoughts, Michael
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: "WARNINGS". 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.
Please fix the issues (WARNINGs) n the R CMD build
and R CMD check
before I can review your package again.
The build report has been available since Jul 12.
http://bioconductor.org/spb_reports/deltaCaptureC_buildreport_20190712095651.html
Hi @michaeldshapiro
I took a look at your package and it's in shape for acceptance.
The man page issues are highly fixable and I suggest you fix them before the package is accepted. The "fixable" warning messages are the only thing stopping me from accepting your package.
Best,
Nitesh
Received a valid push; starting a build. Commits are:
1562da6 Expanded the vignette to include additional exampl...
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, TIMEOUT". 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.
Received a valid push; starting a build. Commits are:
51365e4 Removed evaluation of one example to reduce length...
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". 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.
Received a valid push; starting a build. Commits are:
dbe8c14 Removed an extraneous variable.
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on Linux, Mac, and Windows.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
Your package has been accepted. It will be added to the Bioconductor Git repository and nightly builds. Additional information will be posed to this issue in the next several days.
Thank you for contributing to Bioconductor!
The master branch of your GitHub repository has been added to Bioconductor's git repository.
To use the git.bioconductor.org repository, we need an 'ssh' key to associate with your github user name. If your GitHub account already has ssh public keys (https://github.com/michaeldshapiro.keys is not empty), then no further steps are required. Otherwise, do the following:
See further instructions at
https://bioconductor.org/developers/how-to/git/
for working with this repository. See especially
https://bioconductor.org/developers/how-to/git/new-package-workflow/ https://bioconductor.org/developers/how-to/git/sync-existing-repositories/
to keep your GitHub and Bioconductor repositories in sync.
Your package will be included in the next nigthly 'devel' build (check-out from git at about 6 pm Eastern; build completion around 2pm Eastern the next day) at
https://bioconductor.org/checkResults/
(Builds sometimes fail, so ensure that the date stamps on the main landing page are consistent with the addition of your package). Once the package builds successfully, you package will be available for download in the 'Devel' version of Bioconductor using BiocManager::install("deltaCaptureC")
. The package 'landing page' will be created at
https://bioconductor.org/packages/deltaCaptureC
If you have any questions, please contact the bioc-devel mailing list (https://stat.ethz.ch/mailman/listinfo/bioc-devel); this issue will not be monitored further.
Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor
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:
For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.