Contributions
Contributions copied to clipboard
Damsel
Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor
- Repository: https://github.com/Oshlack/Damsel
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 Bioconductor Package Naming Policy and acknowledge Bioconductor may retain use of package name.
-
[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.
-
[x] I am familiar with the Bioconductor code of conduct and agree to abide by it.
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 questions/help about the submission process, including questions about the output of the automatic reports generated by the SPB (Single Package Builder), please use the #package-submission channel of our Community Slack. Follow the link on the home page of the Bioconductor website to sign up.
Hi @caitlinpage
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: Damsel
Type: Package
Title: Damsel: an end to end analysis of DamID
Version: 0.99.0
Authors@R: person("Caitlin", "Page", email = "[email protected]", role = c("aut", "cre"))
Description: Damsel provides an end to end analysis of DamID data.
Damsel takes bam files from Dam-only control and fusion samples and counts the reads matching to each GATC region. edgeR is utilised to identify regions of enrichment in the fusion relative to the control. Enriched regions are combined into peaks, and are associated with nearby genes.
Damsel allows for IGV style plots to be built as the results build, inspired by ggcoverage, and using the functionality and layering ability of ggplot2.
Damsel also conducts gene ontology testing with bias correction through goseq, and future versions of Damsel will also incorporate motif enrichment analysis.
Overall, Damsel is the first package allowing for an end to end analysis with visual capabilities. The goal of Damsel was to bring all the analysis into one place, and allow for exploratory analysis within R.
License: MIT + file LICENSE
Encoding: UTF-8
Suggests:
biovizBase,
biomaRt,
BSgenome.Dmelanogaster.UCSC.dm6,
knitr,
limma,
org.Dm.eg.db,
rmarkdown,
testthat (>= 3.0.0),
TxDb.Dmelanogaster.UCSC.dm6.ensGene
Config/testthat/edition: 3
Depends:
R (>= 3.50)
RoxygenNote: 7.3.1
Imports:
AnnotationDbi,
Biostrings,
dplyr,
edgeR,
GenomeInfoDb,
GenomicFeatures,
GenomicRanges,
ggbio,
ggplot2,
goseq,
magrittr,
patchwork,
plyranges,
reshape2,
rlang,
Rsamtools,
Rsubread,
stats,
stringr,
tidyr,
utils
BugReports: https://github.com/Oshlack/Damsel
URL: https://github.com/Oshlack/Damsel
biocViews: DifferentialMethylation, PeakDetection, GenePrediction, GeneSetEnrichment
VignetteBuilder: knitr
LazyData: FALSE
In the vignette, please hide the first library(Damsel) as it is currently shown as a random code piece before the vignette starts.
Also, please change the vignette installations to not include a force=TRUE, this will force a re-installation even if the user has the package already installed (and cost Bioconductor egress cost). The correct installation code should be
if (!require("BiocManager", quietly = TRUE))
install.packages("BiocManager")
BiocManager::install("Damsel")
library(Damsel)
Thank you for the feedback. I have now updated the code to show the correct installation.
Your package has been added to git.bioconductor.org to continue the pre-review process. A build report will be posted shortly. Please fix any ERROR and WARNING in the build report before a reviewer is assigned or provide a justification on why you feel the ERROR or WARNING should be granted an exception.
IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. All changes should be pushed to git.bioconductor.org moving forward. It is required to push a version bump to git.bioconductor.org to trigger a new build report.
Bioconductor utilized your github ssh-keys for git.bioconductor.org access. To manage keys and future access you may want to active your Bioconductor Git Credentials Account
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on the Bioconductor Single Package Builder.
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 following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): Damsel_0.99.0.tar.gz
Links above active for 21 days.
Remember: if you submitted your package after July 7th, 2020,
when making changes to your repository push to
[email protected]:packages/Damsel to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Received a valid push on git.bioconductor.org; starting a build for commit id: ba37341a46465a520603c0e1ec90fe8c036a539c
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on the Bioconductor Single Package Builder.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): Damsel_0.99.1.tar.gz
Links above active for 21 days.
Remember: if you submitted your package after July 7th, 2020,
when making changes to your repository push to
[email protected]:packages/Damsel to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.
Package 'Damsel' Review
Thank you for submitting your package to Bioconductor. The package passed check and build. However there are several things need to be fixed. Please try to answer the comments line by line when you are ready for a second review. Code: Note: please consider; Important: must be addressed.
The DESCRIPTION file
- [ ] Important: R version should be no less than 4.4
- [ ] NOTE: Consider adding the maintainer's ORCID iD in 'Authors@R' with 'comment=c(ORCID="...")'
The NAMESPACE file
- [ ] Important: Function names use
camelCaseorsnake_caseand do not include..- in line 13 export(geom_genes.tx)
unacceptable files
- [ ] Important: unacceptable files present (see .gitignore for a listing).
- ./tests/testthat/Rplots.pdf
R code
- [ ] Important:
is()orinherits()instead ofclass().- In file R/gatc_track.R:
- at line 21 found ' if (!class(object) %in% c("BSgenome", "character")) {'
- at line 24 found ' if (class(object) %in% "BSgenome") {'
- at line 27 found ' } else if (class(object) %in% "character") {'
- In file R/plot_gatc_track.R:
- at line 52 found ' while ("patchwork" %in% class(plot2)) {'
- In file R/plot_genes.R:
- at line 59 found ' while ("patchwork" %in% class(plot2)) {'
- In file R/plot_peak.R:
- at line 58 found ' while ("patchwork" %in% class(plot2)) {'
- In file R/plot_region_de_lfc.R:
- at line 40 found ' while ("patchwork" %in% class(plot2)) {'
- In file R/gatc_track.R:
- [ ] NOTE:
::is not suggested in source code unless you can make sure all the packages are imported. Some people think it is better to keep::. However please note that you need to manully double check the import items when you make any change in the DESCRIPTION file during development. My recommendation is to remove one or two repeats to force the dependency check. - [ ] NOTE: Vectorize:
forloops present, try to replace them by*applyfuncitons.- In file R/diff_meth_edgeR.R:
- at line 52 found ' for (i in n_samples) {'
- In file R/helpers.R:
- at line 13 found ' for (i in seq_len(size_n)) {'
- In file R/peak_calling.R:
- at line 172 found ' for (i in number) {'
- at line 178 found ' for (j in (i + 1):nrow(gaps)) {'
- In file R/plot_wrap.R:
- at line 116 found ' for (i in seq_len(length_start)) {'
- In file R/process_bams.R:
- at line 66 found ' for (i in seq_len(length(paired_samples))) {'
- at line 78 found ' for (i in seq_len(length(single_samples))) {'
- at line 97 found ' for (i in seq_len(length(list_files))) {'
- In file R/diff_meth_edgeR.R:
- [ ] Important: Use
file.pathto replacepaste- In file R/process_bams.R:
- at line 51 found ' == "/", path_to_bams, paste0(path_to_bams, "/"))'
- at line 52 found ' list_files <- paste0(path_to_bams, list_files)'
- In file R/process_bams.R:
- [ ] Important: Remove unused code.
- In file R/gene_annotation.R:
- at line 76 found ' # regions$seqnames <- paste0("chr", regions$seqnames)'
- at line 87 found ' # seqnames = ifelse(grepl("chr", seqnames), seqnames, paste0("chr", seqnames))'
- In file R/peak_calling.R:
- at line 63 found ' # gaps <- gaps_new(df = peaks, dm_results = df, gap_size = {{ gap_size }})'
- at line 67 found ' # gaps <- ..peakGaps(df = peaks, dm_results = df, gap_size = {{ gap_size }})'
- In file R/plot_wrap.R:
- at line 90 found ' # id <- id[!is.na(id)]'
- In file R/gene_annotation.R:
- [ ] NOTE: Try to check the edge condition when using
seq.intorseq_len. For example usingseq.int(min(5, nrow(data)))to replaceseq.int(5)- In file R/gene_ontology.R:
- at line 112 found ' df <- signif_results[seq_len(10), ]'
- In file R/gene_annotation.R:
- at line 195 found ' colnames(.[11:ncol(.)])'
- at line 201 found ' colnames(.[11:ncol(.)])'
- at line 213 found ' colnames(.[11:ncol(.)])'
- In file R/peak_calling.R:
- at line 178 found ' for (j in (i + 1):nrow(gaps)) {'
- In file R/gene_ontology.R:
- [ ] NOTE: Functional programming: code repetition.
- repetition in
..addDM, andidentifyPeaks- in ..addDM
- line 1: regions)
- line 2:{
- line 3: if (!is.data.frame(dm_results)) {
- line 4: stop("Must have data frame of differential testing results from `edgeR_results")
- line 5: }
- in identifyPeaks
- line 1: gap_size = 150)
- line 2:{
- line 3: if (!is.data.frame(dm_results)) {
- line 4: stop("Must have data frame of differential_testing results from
edgeR_results") - line 5: }
- in ..addDM
- repetition in
..checkForGaps, and..overlapGaps- in ..checkForGaps
- line 14: gaps$id_list <- character(0)
- line 15: gaps <- gaps[, c("peak_id", "Position", "seqnames", "start",
- line 16: "end", "width", "strand", "logFC_match", "FDR", "multiple_peaks",
- line 17: "n_regions_dm", "n_regions_not_dm", "region_pos",
- line 18: "id_list")]
- line 32: gaps_check <- gaps_check[, c("peak_id", "Position", "seqnames",
- line 33: "start", "end", "width", "strand", "logFC_match",
- line 34: "FDR", "multiple_peaks", "n_regions_dm", "n_regions_not_dm",
- line 35: "region_pos", "id_list")]
- in ..overlapGaps
- line 24: overlap_gaps <- overlap_gaps[, c("peak_id", "Position", "seqnames",
- line 25: "start", "end", "width", "strand", "logFC_match", "FDR",
- line 26: "multiple_peaks", "n_regions_dm", "n_regions_not_dm",
- line 27: "region_pos", "id_list")]
- in ..checkForGaps
- repetition in
..dmReshape, and..plotCountsReshape- in ..dmReshape
- line 1:{
- line 2: df <- df_regions %>% dplyr::mutate(number = seq_len(dplyr::n())) %>%
- line 3: .[rep(seq_len(nrow(.)), times = 4), ] %>% .[order(.$number),
- line 4: ] %>% dplyr::group_by(.data$number) %>% dplyr::mutate(num = seq_len(dplyr::n())) %>%
- line 5: dplyr::mutate(Position = dplyr::case_when(.data$num ==
- line 6: 1 ~ .data$start, .data$num == 2 ~ .data$start, .data$num ==
- line 7: 3 ~ .data$end, TRUE ~ .data$end), y_axis_2 = dplyr::case_when(.data$num ==
- in ..plotCountsReshape
- line 1:{
- line 2: df <- counts %>% dplyr::mutate(number = seq_len(dplyr::n())) %>%
- line 3: .[rep(seq_len(nrow(.)), times = 4), ] %>% .[order(.$number),
- line 4: ] %>% dplyr::group_by(.data$number) %>% dplyr::mutate(num = seq_len(dplyr::n())) %>%
- line 5: dplyr::mutate(Position = dplyr::case_when(.data$num ==
- line 6: 1 ~ .data$start, .data$num == 2 ~ .data$start, .data$num ==
- line 7: 3 ~ .data$end, TRUE ~ .data$end))
- in ..dmReshape
- repetition in
..dmTheme,..peakGatcTheme, and..themeGenePlot- in ..dmTheme
- line 4: angle = 90, vjust = 0.5), axis.text.x = ggplot2::element_blank(),
- line 5: axis.title.x = ggplot2::element_blank(), axis.ticks.x = ggplot2::element_blank(),
- line 6: panel.border = ggplot2::element_rect(colour = "black",
- line 7: fill = NA, linewidth = 1), plot.margin = ggplot2::margin(t = 0.1,
- in ..peakGatcTheme
- line 3: list(ggplot2::theme_classic(), ggplot2::theme(axis.line.y = ggplot2::element_blank(),
- line 4: axis.text.y = ggplot2::element_blank(), axis.title.y.right = ggplot2::element_text(color = "black",
- line 5: angle = 90, vjust = 0.5), axis.ticks.y = ggplot2::element_blank(),
- line 6: axis.text.x = ggplot2::element_blank(), axis.title.x = ggplot2::element_blank(),
- line 7: axis.ticks.x = ggplot2::element_blank(), panel.border = ggplot2::element_rect(colour = "black",
- line 8: fill = NA, linewidth = 1), plot.margin = ggplot2::margin(t = margin.len,
- in ..themeGenePlot
- line 5: position = "right"), ggplot2::theme_classic(), ggplot2::theme(axis.line.y = ggplot2::element_blank(),
- line 6: axis.text.y = ggplot2::element_blank(), axis.title.y.right = ggplot2::element_text(color = "black",
- line 7: angle = 90, vjust = 0.5), axis.ticks.y = ggplot2::element_blank(),
- line 8: axis.text.x = ggplot2::element_blank(), axis.title.x = ggplot2::element_blank(),
- line 9: axis.ticks.x = ggplot2::element_blank(), panel.border = ggplot2::element_rect(colour = "black",
- line 10: fill = NA, linewidth = 1), plot.margin = ggplot2::margin(t = 0.1,
- line 11: b = 0.1)))
- in ..dmTheme
- repetition in
..plotPeak, andggplot_add.genes.tx- in ..plotPeak
- line 2:{
- line 3: if (nrow(valid.bed) == 0) {
- line 4: message("No data available for this region")
- line 5: peak.plot <- ggplot2::ggplot() + ggplot2::geom_blank()
- line 6: }
- line 7: else {
- in ggplot_add.genes.tx
- line 28: if (nrow(df) == 0) {
- line 29: message("No gene data available for this region")
- line 30: gene_plot <- ggplot2::ggplot() + ggplot2::geom_blank()
- line 31: }
- line 32: else {
- in ..plotPeak
- repetition in
ggplot_add.dm,ggplot_add.gatc,ggplot_add.genes.tx, andggplot_add.peak- in ggplot_add.dm
- line 1: plot, object_name)
- line 2:{
- line 3: if (!is.data.frame(object$dm_results.df)) {
- line 4: stop("data.frame of dm results is required")
- line 5: }
- line 6: plot2 <- plot
- line 7: while ("patchwork" %in% class(plot2)) {
- line 8: plot2 <- plot2[[1]]
- line 9: }
- line 10: plot.data <- plot2$labels$title
- line 11: plot.data <- stringr::str_split_1(plot.data, ":")
- line 12: plot.chr <- plot.data[1]
- line 13: plot.data <- stringr::str_split_1(plot.data[2], "-")
- line 14: plot.region.start <- plot.data[1] %>% as.numeric()
- line 15: plot.region.end <- plot.data[2] %>% as.numeric()
- line 16: dm_results.df <- object$dm_results.df
- in ggplot_add.gatc
- line 1: plot, object_name)
- line 2:{
- line 3: if (!is.data.frame(object$gatc_sites.df) && !inherits(object$gatc_sites.df,
- line 4: "GRanges")) {
- line 6: }
- line 7: plot2 <- plot
- line 8: while ("patchwork" %in% class(plot2)) {
- line 9: plot2 <- plot2[[1]]
- line 10: }
- line 11: plot.data <- plot2$labels$title
- line 12: plot.data <- stringr::str_split_1(plot.data, ":")
- line 13: plot.chr <- plot.data[1]
- line 14: plot.data <- stringr::str_split_1(plot.data[2], "-")
- line 15: plot.region.start <- plot.data[1] %>% as.numeric()
- line 16: plot.region.end <- plot.data[2] %>% as.numeric()
- line 17: gatc_sites.df <- object$gatc_sites.df %>% data.frame()
- line 18: gatc.color <- object$gatc.color
- line 19: gatc.size <- object$gatc.size
- line 20: plot.space <- object$plot.space
- line 21: plot.height <- object$plot.height
- line 22: valid.bed <- ..getRegionsPlot(df = gatc_sites.df, columns = c("seqnames",
- line 23: "start", "end"), chr = plot.chr, start = plot.region.start,
- line 24: end = plot.region.end)
- line 25: gatc.plot <- ..plotPeak(valid.bed = valid.bed, plot.size = gatc.size,
- line 26: plot.color = gatc.color, peak.label = FALSE)
- line 27: gatc.plot <- gatc.plot + ggplot2::labs(y = "GATC") + ..peakGatcTheme(margin.len = plot.space,
- line 28: x.range = c(plot.region.start, plot.region.end))
- line 29: patchwork::wrap_plots(plot + ggplot2::theme(plot.margin = ggplot2::margin(t = plot.space,
- line 30: b = plot.space)), gatc.plot, ncol = 1, heights = c(1,
- line 31: plot.height))
- in ggplot_add.genes.tx
- line 1: plot, object_name)
- line 2:{
- line 3: if (!is.data.frame(object$genes.df) && !inherits(object$genes.df,
- line 4: "GRanges")) {
- line 9: }
- line 10: plot2 <- plot
- line 11: while ("patchwork" %in% class(plot2)) {
- line 12: plot2 <- plot2[[1]]
- line 13: }
- line 14: plot.data <- plot2$labels$title
- line 15: plot.data <- stringr::str_split_1(plot.data, ":")
- line 16: plot.chr <- plot.data[1]
- line 17: plot.data <- stringr::str_split_1(plot.data[2], "-")
- line 18: plot.region.start <- plot.data[1] %>% as.numeric()
- line 19: plot.region.end <- plot.data[2] %>% as.numeric()
- line 20: df_og <- object$genes.df %>% data.frame()
- line 45: plot.end = plot.region.end, gene_lim = gene_limits)
- line 46: patchwork::wrap_plots(plot + ggplot2::theme(plot.margin = ggplot2::margin(t = plot.space,
- line 47: b = plot.space)), gene_plot, ncol = 1, heights = c(1,
- line 48: plot.height))
- in ggplot_add.peak
- line 1: plot, object_name)
- line 2:{
- line 3: if (!is.data.frame(object$peaks.df)) {
- line 4: stop("data.frame of peaks is required")
- line 5: }
- line 6: plot2 <- plot
- line 7: while ("patchwork" %in% class(plot2)) {
- line 8: plot2 <- plot2[[1]]
- line 9: }
- line 10: plot.data <- plot2$labels$title
- line 11: plot.data <- stringr::str_split_1(plot.data, ":")
- line 12: plot.chr <- plot.data[1]
- line 13: plot.data <- stringr::str_split_1(plot.data[2], "-")
- line 14: plot.region.start <- plot.data[1] %>% as.numeric()
- line 15: plot.region.end <- plot.data[2] %>% as.numeric()
- line 16: peaks.df <- object$peaks.df
- line 18: peak.color <- object$peak.color
- line 19: peak.size <- object$peak.size
- line 20: plot.space <- object$plot.space
- line 21: plot.height <- object$plot.height
- line 22: valid.bed <- ..getRegionsPlot(df = peaks.df, columns = c("seqnames",
- line 23: "start", "end", "peak_id"), chr = plot.chr, start = plot.region.start,
- line 24: end = plot.region.end)
- line 25: peak.plot <- ..plotPeak(valid.bed = valid.bed, plot.size = peak.size,
- line 26: plot.color = peak.color, peak.label = peak.label)
- line 27: peak.plot <- peak.plot + ggplot2::labs(y = "Peak") + ..peakGatcTheme(margin.len = plot.space,
- line 28: x.range = c(plot.region.start, plot.region.end))
- line 29: patchwork::wrap_plots(plot + ggplot2::theme(plot.margin = ggplot2::margin(t = plot.space,
- line 30: b = plot.space)), peak.plot, ncol = 1, heights = c(1,
- line 31: plot.height))
- in ggplot_add.dm
- repetition in
Documentation
- [ ] Note: Vignette should use
BiocStylepackage for formatting.- rmd file vignettes/Damsel-workflow.Rmd
- [ ] To avoid the '* NOTE: Consider adding runnable examples to man pages that document exported objects.' in BiocCheck, could you please add runnable examples to - pipe.Rd
Received a valid push on git.bioconductor.org; starting a build for commit id: a3d6fa9ac1627de7893ae49b988e9ee5b9e0a6b9
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on the Bioconductor Single Package Builder.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): Damsel_0.99.2.tar.gz
Links above active for 21 days.
Remember: if you submitted your package after July 7th, 2020,
when making changes to your repository push to
[email protected]:packages/Damsel to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Hi @jianhong thank you for the feedback. I've made the following changes:
The DESCRIPTION file
- [x] Important: R version should be no less than 4.4
- [x] NOTE: Consider adding the maintainer's ORCID iD in 'Authors@R' with 'comment=c(ORCID="...")'
The NAMESPACE file
[x] Important: Function names use
camelCaseorsnake_caseand do not include..
- in line 13 export(geom_genes.tx) - The function is now geom_genes_tx
unacceptable files
[x] Important: unacceptable files present (see .gitignore for a listing).
- ./tests/testthat/Rplots.pdf - I've added the file to .git.ignore
R code
- [x] Important:
is()orinherits()instead ofclass(). -I've replaced it with inherits()
- [ ] NOTE:
::is not suggested in source code unless you can make sure all the packages are imported. Some people think it is better to keep::. However please note that you need to manully double check the import items when you make any change in the DESCRIPTION file during development. My recommendation is to remove one or two repeats to force the dependency check. I don't understand your recommendation to remove one or two repeats to force the dependency check.
- [ ] NOTE: Vectorize:
forloops present, try to replace them by*applyfuncitons. I've replaced the loops in the files R/diff_meth_edgeR, R/process_bams. I would prefer to keep the loops in R/helpers, R/peak_calling and R/plot_wrap because I don't know how to vectorise them and gain the same result - particuarly for the loops in R/peak_calling - where the result of the previous very much informs the next result.
- [x] Important: Use
file.pathto replacepasteFile path has replaced paste
- [x] Important: Remove unused code. All the unused code has been removed
- [x] NOTE: Try to check the edge condition when using
seq.intorseq_len. For example usingseq.int(min(5, nrow(data)))to replaceseq.int(5)I've followed this recommendation in R/gene_ontology and fixed this in R/gene_annotation. However, I would like to keep this as it is in R/peak_calling as there is a check in the previous function which acts to check the edge case - this loop is skipped if the edge case occurs.
[ ] NOTE: Functional programming: code repetition.
- repetition in
..addDM, andidentifyPeaks-fixed by removing the unnecessary code in ..addDM
- repetition in
..checkForGaps, and..overlapGapsI would prefer to keep this repetition as it is just the column names and it is easier to read the code with the names there than have to look for the helper functions code.
- repetition in
..dmReshape, and..plotCountsReshape-Fixed by moving the code into another function - ..regionRectangle
- repetition in
..dmTheme,..peakGatcTheme, and..themeGenePlotI would prefer to keep this repetition to avoid having too many helper functions within the plots and getting confused
- repetition in
..plotPeak, andggplot_add.genes.txI would prefer to keep this repetition so that the code in the individual functions is clear and shows that it will handle having no data
- repetition in
ggplot_add.dm,ggplot_add.gatc,ggplot_add.genes.tx, andggplot_add.peakI would prefer to keep this repetition so that each function shows how the data comes in and is processed. Also, as it is used to define various parameters used within the plot, it would be impractical to move this into a helper function - as it wouldn't define the parameters.
Documentation
[x] Note: Vignette should use
BiocStylepackage for formatting.
- rmd file vignettes/Damsel-workflow.Rmd
[x] To avoid the '* NOTE: Consider adding runnable examples to man pages that document exported objects.' in BiocCheck, could you please add runnable examples to
- pipe.Rd
Thank you for your feedback, I look forward to your response.
Received a valid push on git.bioconductor.org; starting a build for commit id: 1a1a9b53918f79e8e70de2aef854d9e7cb3856bf
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on the Bioconductor Single Package Builder.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): Damsel_0.99.3.tar.gz
Links above active for 21 days.
Remember: if you submitted your package after July 7th, 2020,
when making changes to your repository push to
[email protected]:packages/Damsel to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Package 'Damsel' Review
In current review, something about reuse the Bioconductor functions. Please try to answer the comments line by line when you are ready for a second review. Code: Note: please consider; Important: must be addressed.
unacceptable files
- [ ] Important: please remove the file
- ./tests/testthat/Rplots.pdf
General package development
- [ ] Note: Consider adding these automatically suggested biocViews: MotifAnnotation
R code
- [ ] Note: try to understand more usage cases for
is()andinherits(). - [ ] NOTE: try to fix the seqnames with GenomeInfoDb::seqlevelsStyle
- [ ] Important: Please consider to add
drop=FALSEto avoid the reduction of dimension for matrices and arrays. Ignore this if using datatable.- In file R/gatc_track.R:
- at line 50 found ' df <- df[, c("Position", "seqnames", "start", "end", "width")]'
- In file R/gene_annotation.R:
- at line 30 found ' genes_ <- data.frame(genes_)[, c("seqnames", "start", "end", "width", "strand", "ensembl_gene_id")]'
- at line 144 found ' n_regions <- combo[, c("seqnames", "start", "end", "width", "num")]'
- at line 275 found ' df <- df[, col_order]'
- In file R/helpers.R:
- at line 21 found ' df[, c("Position", "seqnames", "start", "end", "width", "strand")]'
- In file R/plot_helper_fns.R:
- at line 10 found ' df.select <- df[df$end >= start & df$start <= end, ]'
- In file R/process_bams.R:
- at line 36 found ' regions <- regions[,c("Position", "seqnames", "start", "end", "width", "strand")]'
- In file R/gatc_track.R:
- [ ] Important: Additional input check should be involved in
makeDGEforcounts.df,countBamInGATCforregions.
Documentation
- [ ] Note: Typos: paramter identifyPeaks.Rd:21
Received a valid push on git.bioconductor.org; starting a build for commit id: 8b9627ea100f8bf41208ef4a4adf085fedf21058
Dear Package contributor,
This is the automated single package builder at bioconductor.org.
Your package has been built on the Bioconductor Single Package Builder.
Congratulations! The package built without errors or warnings on all platforms.
Please see the build report for more details.
The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): Damsel_0.99.4.tar.gz
Links above active for 21 days.
Remember: if you submitted your package after July 7th, 2020,
when making changes to your repository push to
[email protected]:packages/Damsel to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Hi @jianhong thank you for your review.
Package 'Damsel' Review
In current review, something about reuse the Bioconductor functions. Please try to answer the comments line by line when you are ready for a second review. Code: Note: please consider; Important: must be addressed.
unacceptable files
[x] Important: please remove the file
- ./tests/testthat/Rplots.pdf This file has now been removed
General package development
- [] Note: Consider adding these automatically suggested biocViews: MotifAnnotation Damsel conducts gene ontology testing but does not actually do motif analysis, so I would not like to add the MotifAnnotation BiocView term.
R code
[ ] Note: try to understand more usage cases for
is()andinherits(). I believe the extra checks I have made help with this goal, and this is something I will continue to learn and build on as my coding experience grows.[x] NOTE: try to fix the seqnames with GenomeInfoDb::seqlevelsStyle I have fixed some of these cases with a new function I made, in particular in areas that make sense within the code. There are a few I would like to leave as is, because they have been set earlier in the function with this new method, and in order to change them again in GenomeInfoDb I would need to change the columns and then rearrange them after to have the settings I like.
[x] Important: Please consider to add
drop=FALSEto avoid the reduction of dimension for matrices and arrays. Ignore this if using datatable.
In file R/gatc_track.R:
- at line 50 found ' df <- df[, c("Position", "seqnames", "start", "end", "width")]'
In file R/gene_annotation.R:
- at line 30 found ' genes_ <- data.frame(genes_)[, c("seqnames", "start", "end", "width", "strand", "ensembl_gene_id")]'
- at line 144 found ' n_regions <- combo[, c("seqnames", "start", "end", "width", "num")]'
- at line 275 found ' df <- df[, col_order]'
In file R/helpers.R:
- at line 21 found ' df[, c("Position", "seqnames", "start", "end", "width", "strand")]'
In file R/plot_helper_fns.R:
- at line 10 found ' df.select <- df[df$end >= start & df$start <= end, ]'
In file R/process_bams.R:
- at line 36 found ' regions <- regions[,c("Position", "seqnames", "start", "end", "width", "strand")]'
[x] Important: Additional input check should be involved in
makeDGEforcounts.df,countBamInGATCforregions. I have made additional checks for some of the columns used by those functions.Documentation
- [x] Note: Typos: paramter identifyPeaks.Rd:21
Thank you again for your time and your feedback, I look forward to your response.
Your package has been accepted. It will be added to the Bioconductor nightly builds.
Thank you for contributing to Bioconductor!
Reviewers for Bioconductor packages are volunteers from the Bioconductor community. If you are interested in becoming a Bioconductor package reviewer, please see Reviewers Expectations.
The default branch of your GitHub repository has been added to Bioconductor's git repository as branch devel.
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/caitlinpage.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("Damsel"). The package 'landing page' will be created at
https://bioconductor.org/packages/Damsel
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.