Contributions
Contributions copied to clipboard
Bioconductor
squallms
Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor
- Repository: https://github.com/wkumler/squallms
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 mass-spectrometry 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 @wkumler
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: squallms
Type: Package
Title: Speedy quality assurance via lasso labeling for LC-MS data
Version: 0.99.0
Authors@R: c(
person(given = "William", family = "Kumler", email = "[email protected]",
role = c("aut", "cre", "cph"), comment=c(ORCID="0000-0002-5022-8009"))
)
Description: squallms is a Bioconductor R package that implements a
"semi-labeled" approach to untargeted mass spectrometry data. It pulls in raw
data from mass-spec files to calculate several metrics that are then used to
label MS features in bulk as high or low quality. These metrics of peak quality
are then passed to a simple logistic model that produces a fully-labeled
dataset suitable for downstream analysis.
License: MIT + file LICENSE
URL: https://github.com/wkumler/squallms
BugReports: https://github.com/wkumler/squallms/issues
Encoding: UTF-8
RoxygenNote: 7.3.1
biocViews: MassSpectrometry, Metabolomics, Proteomics, Lipidomics, ShinyApps,
Classification, Clustering, FeatureExtraction, PrincipalComponent, Regression,
Preprocessing, QualityControl, Visualization
Depends:
R (>= 3.5.0)
Imports:
xcms,
MSnbase,
RaMS,
dplyr,
tidyr,
tibble,
ggplot2,
shiny,
plotly,
data.table,
caret,
stats,
graphics,
grDevices,
utils
Suggests:
knitr,
rmarkdown,
BiocStyle,
testthat (>= 3.0.0)
VignetteBuilder: knitr
Config/testthat/edition: 3
Please include an inst/scripts that has a file that describes how the data in inst/extdata was generated. It can be text, psuedo-code, or code but should contain any relevant source and licensing information.
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: "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.
The following are build products from R CMD build on the Single Package Builder: ERROR before build products produced.
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/squallms 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: 9115a56d52d5371613242f22093329a3208a7f5e
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): squallms_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/squallms 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: f3c495c697f29acb0665a6e1ce8a73b11cec492e
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): squallms_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/squallms 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: d62e83477270529f4f08b48b98f696875cc544e4
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): squallms_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/squallms 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.
Hi @wkumler,
Just a heads up that I won't begin the review until after the forthcoming BioC 3.19 release (scheduled for May 1). This is because the package was submitted after the deadline for inclusion in BioC 3.19 (hence the 'Post Release Deadline' tag on this issue).
Thanks for your understanding, Pete
Hi Pete,
Yeah, I figured! Was hoping to sneak in under the code freeze deadline but totally understandable that you'd like to give it more review time. I appreciate the work you put in on Bioconductor!
-Will
Hi @wkumler,
Thank you for submitting squallms to Bioconductor and thanks for your patience awaiting this review.
Overall, the package is in good shape and close to being ready for acceptance. My main suggestions are around that I think the vignette could be improved to make it friendlier to new users. For context, I say this as someone who does not do any analysis of mass spectrometry data but who is familiar with the general concepts.
In my review below I have separated the issues into Required and Recommended points that I would ask you to address before the package can be accepted. Would you please provide line-by-line comments to my initial review so that I know what changes I'm looking for in my re-review.
Cheers, Pete
Required
- [ ] The vignette needs an Introduction section. Currently, the 'Peakpicking with XCMS' section of vignette sort of serves that, but it throws a new user in the deep end. There's some content in the README that could be included to provide some more background and introduction to the subject area and package's purpose.
- [ ] Remove any commented out code and "Not run" code from the vignette (why is it there if it's not run?).
- [ ] Please include more description of the purpose and output of each code chunk in the vignette. E.g., before the code chunk explain what it's going to do and afterwards explain what the output is.
- [ ] All tables and figures should include a caption (see https://bioconductor.org/packages/release/bioc/vignettes/BiocStyle/inst/doc/AuthoringRmdVignettes.html#62_Figure_captions). More generally, all tables and plots in vignette need more explanation (e.g., what do column names or axis names mean, how to interpret the plot). E.g., the first plot has no explanation leading up to it and the text following it says "So far so good. Unfortunately, many of these features are very low-quality ... " but how do I know this from the plot?
- [ ] Please minimise the use of unevaluated code chunks in the vignette where possible.
- [ ]
labelFeatsManual()did not work on my system (session info below). What computers or operating systems have you tested this on?
# Using the example from `?labelFeatsManual
> library(squallms)
> library(xcms)
> library(dplyr)
> library(MSnbase)
> mzML_files <- system.file("extdata", package = "RaMS") %>%
+ list.files(full.names = TRUE, pattern = "[A-F].mzML")
> register(BPPARAM = SerialParam())
> cwp <- CentWaveParam(snthresh = 0, extendLengthMSW = TRUE, integrate = 2)
> obp <- ObiwarpParam(binSize = 0.1, response = 1, distFun = "cor_opt")
> pdp <- PeakDensityParam(
+ sampleGroups = 1:3, bw = 12, minFraction = 0,
+ binSize = 0.001, minSamples = 0
+ )
> xcms_filled <- mzML_files %>%
+ readMSData(msLevel. = 1, mode = "onDisk") %>%
+ findChromPeaks(cwp) %>%
+ adjustRtime(obp) %>%
+ groupChromPeaks(pdp) %>%
+ fillChromPeaks(FillChromPeaksParam(ppm = 5))
Detecting mass traces at 25 ppm ... OK
Detecting chromatographic peaks in 198 regions of interest ... OK: 147 found.
Detecting mass traces at 25 ppm ... OK
Detecting chromatographic peaks in 232 regions of interest ... OK: 166 found.
Detecting mass traces at 25 ppm ... OK
Detecting chromatographic peaks in 218 regions of interest ... OK: 176 found.
Sample number 2 used as center sample.
Aligning LB12HL_AB.mzML.gz against LB12HL_CD.mzML.gz ... OK
Aligning LB12HL_EF.mzML.gz against LB12HL_CD.mzML.gz ... OK
Applying retention time adjustment to the identified chromatographic peaks ... OK
[===============================================================================================================================================================] 100/100 (100%) in 0s
Defining peak areas for filling-in .... OK
Start integrating peak areas from original files
Requesting 57 peaks from LB12HL_AB.mzML.gz ... got 52.
Requesting 46 peaks from LB12HL_CD.mzML.gz ... got 45.
Requesting 41 peaks from LB12HL_EF.mzML.gz ... got 40.
> peak_data <- makeXcmsObjFlat(xcms_filled)
> if(interactive()){
+ manual_labels <- labelFeatsManual(peak_data)
+ }
Grabbing raw MS1 data
|===============================================================================================================================================================================| 100%
Total time: 0.32 secs
Error in setGraphicsEventEnv(which, as.environment(list(...))) :
this graphics device does not support event handling
Session info
sessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#> setting value
#> version R version 4.4.0 (2024-04-24)
#> os macOS Sonoma 14.4.1
#> system aarch64, darwin20
#> ui X11
#> language (EN)
#> collate en_US.UTF-8
#> ctype en_US.UTF-8
#> tz Australia/Melbourne
#> date 2024-05-16
#> pandoc 3.1.11 @ /Applications/RStudio.app/Contents/Resources/app/quarto/bin/tools/aarch64/ (via rmarkdown)
#>
#> ─ Packages ───────────────────────────────────────────────────────────────────
#> ! package * version date (UTC) lib source
#> P abind 1.4-5 2016-07-21 [?] CRAN (R 4.4.0)
#> P affy 1.83.0 2024-05-04 [?] Bioconduc~
#> P affyio 1.75.0 2024-05-04 [?] Bioconduc~
#> P AnnotationFilter 1.29.0 2024-05-04 [?] Bioconduc~
#> P Biobase * 2.65.0 2024-05-04 [?] Bioconduc~
#> P BiocGenerics * 0.51.0 2024-05-04 [?] Bioconduc~
#> P BiocManager 1.30.23 2024-05-04 [?] CRAN (R 4.4.0)
#> P BiocParallel * 1.39.0 2024-05-04 [?] Bioconduc~
#> P cli 3.6.2 2023-12-11 [?] CRAN (R 4.4.0)
#> P clue 0.3-65 2023-09-23 [?] CRAN (R 4.4.0)
#> P cluster 2.1.6 2023-12-01 [3] CRAN (R 4.4.0)
#> P codetools 0.2-20 2024-03-31 [3] CRAN (R 4.4.0)
#> P colorspace 2.1-0 2023-01-23 [?] CRAN (R 4.4.0)
#> P crayon 1.5.2 2022-09-29 [?] CRAN (R 4.4.0)
#> P DBI 1.2.2 2024-02-16 [?] CRAN (R 4.4.0)
#> P DelayedArray 0.31.1 2024-05-07 [?] Bioconduc~
#> P digest 0.6.35 2024-03-11 [?] CRAN (R 4.4.0)
#> P doParallel 1.0.17 2022-02-07 [?] CRAN (R 4.4.0)
#> P dplyr * 1.1.4 2023-11-17 [?] CRAN (R 4.4.0)
#> P evaluate 0.23 2023-11-01 [?] CRAN (R 4.4.0)
#> P fansi 1.0.6 2023-12-08 [?] CRAN (R 4.4.0)
#> P fastmap 1.1.1 2023-02-24 [?] CRAN (R 4.4.0)
#> P foreach 1.5.2 2022-02-02 [?] CRAN (R 4.4.0)
#> P fs 1.6.4 2024-04-25 [?] CRAN (R 4.4.0)
#> P generics 0.1.3 2022-07-05 [?] CRAN (R 4.4.0)
#> P GenomeInfoDb 1.41.0 2024-05-04 [?] Bioconduc~
#> P GenomeInfoDbData 1.2.12 2024-05-15 [?] Bioconductor
#> P GenomicRanges 1.57.0 2024-05-04 [?] Bioconduc~
#> P ggplot2 3.5.1 2024-04-23 [?] CRAN (R 4.4.0)
#> P glue 1.7.0 2024-01-09 [?] CRAN (R 4.4.0)
#> P gtable 0.3.5 2024-04-22 [?] CRAN (R 4.4.0)
#> P hms 1.1.3 2023-03-21 [?] CRAN (R 4.4.0)
#> P htmltools 0.5.8.1 2024-04-04 [?] CRAN (R 4.4.0)
#> P httr 1.4.7 2023-08-15 [?] CRAN (R 4.4.0)
#> P igraph 2.0.3 2024-03-13 [?] CRAN (R 4.4.0)
#> P impute 1.79.0 2024-05-04 [?] Bioconduc~
#> P IRanges 2.39.0 2024-05-04 [?] Bioconduc~
#> P iterators 1.0.14 2022-02-05 [?] CRAN (R 4.4.0)
#> P jsonlite 1.8.8 2023-12-04 [?] CRAN (R 4.4.0)
#> P knitr 1.46 2024-04-06 [?] CRAN (R 4.4.0)
#> P lattice 0.22-6 2024-03-20 [3] CRAN (R 4.4.0)
#> P lazyeval 0.2.2 2019-03-15 [?] CRAN (R 4.4.0)
#> P lifecycle 1.0.4 2023-11-07 [?] CRAN (R 4.4.0)
#> P limma 3.61.0 2024-05-04 [?] Bioconduc~
#> P magrittr 2.0.3 2022-03-30 [?] CRAN (R 4.4.0)
#> P MALDIquant 1.22.2 2024-01-22 [?] CRAN (R 4.4.0)
#> P MASS 7.3-60.2 2024-04-24 [3] local
#> P MassSpecWavelet 1.71.0 2024-05-04 [?] Bioconduc~
#> P Matrix 1.7-0 2024-03-22 [3] CRAN (R 4.4.0)
#> P MatrixGenerics 1.17.0 2024-05-04 [?] Bioconduc~
#> P matrixStats 1.3.0 2024-04-11 [?] CRAN (R 4.4.0)
#> P MetaboCoreUtils 1.13.0 2024-05-04 [?] Bioconduc~
#> P MsCoreUtils 1.17.0 2024-05-04 [?] Bioconduc~
#> P MsExperiment 1.7.0 2024-05-04 [?] Bioconduc~
#> P MsFeatures 1.13.0 2024-05-04 [?] Bioconduc~
#> P MSnbase * 2.31.0 2024-05-04 [?] Bioconduc~
#> P MultiAssayExperiment 1.31.1 2024-05-04 [?] Bioconduc~
#> P munsell 0.5.1 2024-04-01 [?] CRAN (R 4.4.0)
#> P mzID 1.43.0 2024-05-04 [?] Bioconduc~
#> P mzR * 2.39.0 2024-05-04 [?] Bioconduc~
#> P ncdf4 1.22 2023-11-28 [?] CRAN (R 4.4.0)
#> P pcaMethods 1.97.0 2024-05-04 [?] Bioconduc~
#> P pillar 1.9.0 2023-03-22 [?] CRAN (R 4.4.0)
#> P pkgconfig 2.0.3 2019-09-22 [?] CRAN (R 4.4.0)
#> P plyr 1.8.9 2023-10-02 [?] CRAN (R 4.4.0)
#> P preprocessCore 1.67.0 2024-05-04 [?] Bioconduc~
#> P prettyunits 1.2.0 2023-09-24 [?] CRAN (R 4.4.0)
#> P progress 1.2.3 2023-12-06 [?] CRAN (R 4.4.0)
#> P ProtGenerics * 1.37.0 2024-05-04 [?] Bioconduc~
#> P PSMatch 1.9.0 2024-05-04 [?] Bioconduc~
#> P purrr 1.0.2 2023-08-10 [?] CRAN (R 4.4.0)
#> P QFeatures 1.15.1 2024-05-11 [?] Bioconduc~
#> P R6 2.5.1 2021-08-19 [?] CRAN (R 4.4.0)
#> P RColorBrewer 1.1-3 2022-04-03 [?] CRAN (R 4.4.0)
#> P Rcpp * 1.0.12 2024-01-09 [?] CRAN (R 4.4.0)
#> P reprex 2.1.0 2024-01-11 [?] CRAN (R 4.4.0)
#> P reshape2 1.4.4 2020-04-09 [?] CRAN (R 4.4.0)
#> P rlang 1.1.3 2024-01-10 [?] CRAN (R 4.4.0)
#> P rmarkdown 2.26 2024-03-05 [?] CRAN (R 4.4.0)
#> P rstudioapi 0.16.0 2024-03-24 [?] CRAN (R 4.4.0)
#> P S4Arrays 1.5.0 2024-05-04 [?] Bioconduc~
#> P S4Vectors * 0.43.0 2024-05-04 [?] Bioconduc~
#> P scales 1.3.0 2023-11-28 [?] CRAN (R 4.4.0)
#> P sessioninfo 1.2.2 2021-12-06 [?] CRAN (R 4.4.0)
#> P SparseArray 1.5.3 2024-05-11 [?] Bioconduc~
#> P Spectra 1.15.0 2024-05-04 [?] Bioconduc~
#> P statmod 1.5.0 2023-01-06 [?] CRAN (R 4.4.0)
#> P stringi 1.8.4 2024-05-06 [?] CRAN (R 4.4.0)
#> P stringr 1.5.1 2023-11-14 [?] CRAN (R 4.4.0)
#> P SummarizedExperiment 1.35.0 2024-05-04 [?] Bioconduc~
#> P tibble 3.2.1 2023-03-20 [?] CRAN (R 4.4.0)
#> P tidyr 1.3.1 2024-01-24 [?] CRAN (R 4.4.0)
#> P tidyselect 1.2.1 2024-03-11 [?] CRAN (R 4.4.0)
#> P UCSC.utils 1.1.0 2024-05-04 [?] Bioconduc~
#> P utf8 1.2.4 2023-10-22 [?] CRAN (R 4.4.0)
#> P vctrs 0.6.5 2023-12-01 [?] CRAN (R 4.4.0)
#> P vsn 3.73.0 2024-05-04 [?] Bioconduc~
#> P withr 3.0.0 2024-01-16 [?] CRAN (R 4.4.0)
#> P xcms * 4.3.0 2024-05-01 [?] Bioconduc~
#> P xfun 0.43 2024-03-25 [?] CRAN (R 4.4.0)
#> P XML 3.99-0.16.1 2024-01-22 [?] CRAN (R 4.4.0)
#> P XVector 0.45.0 2024-05-04 [?] Bioconduc~
#> P yaml 2.3.8 2023-12-11 [?] CRAN (R 4.4.0)
#> P zlibbioc 1.51.0 2024-05-04 [?] Bioconduc~
#>
#> [1] /Users/hickey/Library/Caches/org.R-project.R/R/renv/library/squallms-b78d0ce3/macos/R-4.4/aarch64-apple-darwin20
#> [2] /Users/hickey/Library/Caches/org.R-project.R/R/renv/sandbox/macos/R-4.4/aarch64-apple-darwin20/f7156815
#> [3] /Library/Frameworks/R.framework/Versions/4.4-arm64/Resources/library
#>
#> P ── Loaded and on-disk path mismatch.
#>
#> ──────────────────────────────────────────────────────────────────────────────
-
[ ] When using
labelFeatsLasso()it's not clear to me what's being plotted in the bottom panels because there are no axis labels. -
[ ] In the 'Lasso labeling with labelFeatsLasso' section of the vignette:
- [ ] How are first and second figures made? There's no code shown in the rendered vignette. Furthermore, both figures need captions.
- [ ] The caret section comes out of nowhere and lacks explanation.
-
The vignette makes use of pre-saved results (in
inst/extdata) but the user is not shown how or when these are loaded in the vignette. Consequently, it's impossible for the user to replicate the vignette output by following the vignette. I understand the necessity of using saved results for some of the interactive selections, but you need to pull back the curtain to explain why this is being done and how. Otherwise the chunks following these selection are not reproducible unless the user has the exact samelasso_classesandmanual_classesvariables as you created. -
[ ] In the 'Building a quality model and removing low-quality peaks' section of vignette.
- [ ] The results are not reproducible because of the aforementioned use of presaved hidden variables.
- [ ] Why the
if (interactive())saving of plot in the vignette?
-
[ ] The
verbosityargument ofextractChromMetrics()doesn't seem to work as documented (see below):
suppressPackageStartupMessages(library(squallms))
suppressMessages(example("extractChromMetrics", "squallms", echo = FALSE))
# Where are the plots that are supposed to appear with verbosity = 2?
feat_metrics <- extractChromMetrics(peak_data, recalc_betas = TRUE, verbosity = 2)
#> Grabbing raw MS1 data
#> | | | 0%
#> Reading file LB12HL_AB.mzML.gz... 0.07 secs
#> Reading MS1 data...0.04 secs
#> | |======================= | 33%
#> Reading file LB12HL_CD.mzML.gz... 0.07 secs
#> Reading MS1 data...0.04 secs
#> | |=============================================== | 67%
#> Reading file LB12HL_EF.mzML.gz... 0.13 secs
#> Reading MS1 data...0.04 secs
#> | |======================================================================| 100%
#> Binding files together into single data.table
#> Total time: 0.39 secs
#> Recalculating beta coefficients
- [ ] The example in
?logModelFeatProbshould explain why there is an RDS file is loaded and what it contains. Does this example really need dplyr (could use of|>instead of%>%simplify it?)?
Recommended
- [ ] Good job providing some unit tests. Have a think if it's possible to test code
R/labelFeatsFunctions.R(seecovr::report()for other places lacking coverage). - [ ] Can you use the base R pipe operator
|>instead of the magrittr pipe operator%>%? - [ ] Please explain in the vignette (perhaps just as a code comment) why there is a need to
set.seed()in certain chunks. - [ ] In the 'Calculating metrics of peak quality (extractChromMetrics)' section of vignette.
- [ ] "BMC Bioinformatics paper I put out in 2023"; recommend including the actual citation (see https://bookdown.org/yihui/rmarkdown-cookbook/bibliography.html).
- [ ] Recommend adding a
inst/CITATIONfile (see https://contributions.bioconductor.org/citation.html) - [ ] Consider adding a package man page (see https://contributions.bioconductor.org/docs.html#package-level-documentation)
- [ ] Consider the NOTES raised by
BiocCheck()::BiocCheck(). These are only suggestions but are worth consideration.
Hi @PeteHaitch, Thank you for the detailed review and thoughtful comments on the package! I have implemented essentially all of your recommendations but could use some advice on the last few.
Required
- The vignette needs an Introduction section.
- Done! Introduction has been added and vignette as a whole expanded significantly.
- Remove any commented out code and "Not run" code from the vignette.
- Done!
- Please include more description of the purpose and output of each code chunk in the vignette.
- Done!
- Please minimise the use of unevaluated code chunks in the vignette where possible.
- Done! The only unevaluated chunks now are those that trigger an interactive option
- All tables and figures should include a caption
- Done!
- labelFeatsManual() did not work on my system (session info below). What computers or operating systems have you tested this on?
- This is a tough one. I tested on Windows and Linux but apparently Macs have problems with X11 graphics. I wasn't able to find any good workarounds for this in the time I spent on the project - it seems possible that calling X11() instead of dev.new() to start a graphics device with event handling might work if XQuartz is installed (not by default for Macs since 2016) but I'd rather not add a hardware dependency like that if possible. At this point, it seems like my options are making it clear that this function is limited to Windows/Linux systems or reformatting the thing into a Shiny app like labelFeatsLasso currently is. Is there an option preferred by Bioconductor? Perhaps noting that Macs will be unable to use this (and adding a warning) is an acceptable compromise for now with intent to restructure into a Shiny app when I've got more bandwidth?
- When using labelFeatsLasso() it's not clear to me what's being plotted in the bottom panels
- I've tried to expand on the bottom panels in the figure caption and the vignette, so let me know if this is still insufficient. I've left those off both for rendering speed and visual cleanliness and am inclined to do so because extracted ion chromatograms are very familiar to anyone working with LCMS data.
- How are first and second figures made in the 'Lasso labeling' section of the vignette?
- Done! Code has been exposed. Makes the vignette a bit less clean but I agree that it's better to be explicit.
- The caret section comes out of nowhere and lacks explanation in the 'Lasso labeling' section of the vignette?
- Done! Added context.
- The vignette makes use of pre-saved results (in inst/extdata) but the user is not shown how or when these are loaded in the vignette.
- Done! Code for reading these results in has been exposed and motivation shared in context.
- The results are not reproducible because of the aforementioned use of presaved hidden variables in the 'Building a quality model' section of vignette.
- Done! I'm hoping this is clearer now.
- Why the if (interactive()) saving of plot in the vignette in the 'Building a quality model' section of vignette?
- Done! These have been removed (mostly just sped up vignette rendering time during development)
- The verbosity argument of extractChromMetrics() doesn't seem to work as documented
- Done! Documentation has been rephrased to make it clear that plots are not always returned
- The example in ?logModelFeatProb should explain why there is an RDS file is loaded and what it contains.
- Done!
- Does this example really need dplyr (could use of |> instead of %>% simplify it?)?
- Done! Switched to base R pipe here.
Recommended
- Good job providing some unit tests. Have a think if it's possible to test code R/labelFeatsFunctions.R (see covr::report() for other places lacking coverage).
- Another toughie. I struggled to write unit tests for the interactive sections for obvious reasons, but they clearly would've caught the X11 issue described above so they might be worth implementing a little more carefully.
- Can you use the base R pipe operator |> instead of the magrittr pipe operator %>%?
- This is possible but a bit of a headache. Is there a reason to prefer the base R version when I'm already loading dplyr for other functions?
- Please explain in the vignette why there is a need to set.seed() in certain chunks.
- Done!
- "BMC Bioinformatics paper I put out in 2023"; recommend including the actual citation in the 'Calculating metrics' section of vignette.
- Done!
- Recommend adding a inst/CITATION file
- Done!
- Consider adding a package man page
- Done!
- Consider the NOTES raised by BiocCheck::BiocCheck()
- Done! I've double-checked on the notes provided by this check and am confident that most recommendations can be safely ignored. I did want to ask about the recommendation to bump the R version dependency from 3.5.0 to 4.4.0 - I realize that's necessary for Bioconductor but I'd rather folks be able to use my package (e.g. directly from Github) without requiring the latest version.
Received a valid push on git.bioconductor.org; starting a build for commit id: acd422f235dbcc938ad7285d587197f6966ca19f
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): squallms_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/squallms to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Hi @wkumler,
Thank you for making the requested changes and considering the recommended ones. I think the package is much improved, particularly the vignette which is much easier for new users with the new intro, figure/table captions, better reproducibility, and better and more detailed text explanations and guides to interpretation. It's very nearly ready for acceptance.
First, some responses to your queries/comments.
- labelFeatsManual() did not work on my system (session info below). What computers or operating systems have you tested this on?
- This is a tough one. I tested on Windows and Linux but apparently Macs have problems with X11 graphics. I wasn't able to find any good workarounds for this in the time I spent on the project - it seems possible that calling X11() instead of dev.new() to start a graphics device with event handling might work if XQuartz is installed (not by default for Macs since 2016) but I'd rather not add a hardware dependency like that if possible. At this point, it seems like my options are making it clear that this function is limited to Windows/Linux systems or reformatting the thing into a Shiny app like labelFeatsLasso currently is. Is there an option preferred by Bioconductor? Perhaps noting that Macs will be unable to use this (and adding a warning) is an acceptable compromise for now with intent to restructure into a Shiny app when I've got more bandwidth?
I think documenting the potential workaround on macOS (see below, and trying to get the backspace and escape keybindings working on macOS) is an okay interim measure until/if you re-implement as a shiny app. I'll ask @Bioconductor/core to chime in whether platform-specific functionality is okay in this case. That said, I was able to partially get it to work on my macOS system with the following (please see the comments below for issues I still had):
library(squallms)
example("labelFeatsManual", echo = FALSE)
# Either of these options worked the same for me.
# options(device = function() X11(type = "Xlib"))
options(device = function() X11(type = "cairo"))
# Left, right, up keys work as expected.
# However, backspace ('delete' on macOS keyboard) and escape ('esc') gave 'Warning in labelFeatsManual(peak_data) : Unrecognized input, skipping'.
# It also sometimes crashed my R session when trying to stop labelling (e.g. by clicking stop in Rstudio or closing graphics window).
manual_labels <- labelFeatsManual(peak_data)
- Good job providing some unit tests. Have a think if it's possible to test code R/labelFeatsFunctions.R (see covr::report() for other places lacking coverage).
- Another toughie. I struggled to write unit tests for the interactive sections for obvious reasons, but they clearly would've caught the X11 issue described above so they might be worth implementing a little more carefully.
That's fair enough. Know that upon acceptance that your package will be built and checked on Windows (x86_64), macOS (x86_64 and arm64), and Linux (Ubuntu; x86_64 and arm64), so that might help you check cross-platform functionality on systems you don't have access to.
- Can you use the base R pipe operator |> instead of the magrittr pipe operator %>%?
- This is possible but a bit of a headache. Is there a reason to prefer the base R version when I'm already loading dplyr for other functions?
It's mostly personal preference (I like to use things already included in R before using things from packages), so it's fine to retain as is.
I did want to ask about the recommendation to bump the R version dependency from 3.5.0 to 4.4.0 - I realize that's necessary for Bioconductor but I'd rather folks be able to use my package (e.g. directly from Github) without requiring the latest version.
You can keep it as is.
But please remember that Bioconductor strongly recommend the primary installation instructions be BiocManager::install("squallms") rather than installing from Github.
In particular, using BiocManager::install("squallms") will ensure the user has/gets the appropriate versions of all dependencies whereas our experience is that users installing from GitHub can often lead to incompatible software dependencies from different versions of Bioconductor and elsewhere.
Second, there's a a small number of new minor things to fix.
Required
- [x] Check formatting of figure captions (e.g., there's a literal
#fig:plot metricsin rendered vignette). In this case, chunk labels should not contain spaces, so you'll want to use something likeplot-metricsinstead ofplot metrics(see https://yihui.org/knitr/options/#chunk-options). - [x] In the vignette, please note with a comment in the corresponding code chunk or text nearby any
eval=FALSEchunks, e.g., themanual_classes <- labelFeatsManual(feat_peak_info, ms1_data = msdata$MS1, selection = "Labeled", existing_labels = lasso_classes)- [x] Furthermore, for this specific chunk, if the user runs it then they will have a different
manual_classesvector for subsequent steps, which might not be ideal because subsequent steps will give different results, so perhaps note that?
- [x] Furthermore, for this specific chunk, if the user runs it then they will have a different
Recommended
- [x] You can opt to not 'echo' the code chunks in the vignette that just load an image (e.g.,
knitr::include_graphics(file.path(here::here(), "vignettes", "intro_good_ss.png"), rel_path = FALSE). - [x] Re "This function also notes squallms in the object’s processing history (albeit poorly)." in the vignette; Can you explain/highlight where in the output we can see this history?
Received a valid push on git.bioconductor.org; starting a build for commit id: e75460cfda53bdefff030cdfb9808cdd8877b9f9
Hi @PeteHaitch,
Thanks again for the thoughtful commentary and support during this submission process! I've now submitted a new version of the package in which I bite the bullet and pivot the labelFeatsManual function into a Shiny app to avoid having to deal with the platform-specific problems. I was able to test this app on both my own Windows laptop and a colleague's Mac so I'm optimistic about your experience with it now.
I agree about the best user experience likely coming from the BiocManager::install("squallms") instead of a Github install and will change that part of the README once the package is accepted. However, I have left the hard dependency at 3.5.0 so that advanced users will be able to use older versions if necessary.
Both Required bullets have been resolved - nice catch with the #fig: problem, you were correct about it being due to a space in the chunk name. I've added code comments to both places where I call an interactive chunk with eval=FALSE and expanded a bit of the text nearby to hopefully provide some additional context as well.
Both Recommended bullets have been resolved - I ended up removing the line about the XCMS processing history and the associated functionality until https://github.com/sneumann/xcms/issues/735 can be better resolved.
Thanks @wkumler. FYI I'm at a conference next week, but I hope to find time to quickly go through these changes and accept the package but apologies if I'm delayed.
The new shiny app worked a charm (with one exception, below). Sorry, a few small things to fix following those changes:
- [x]
manual_relabel_lassochunk needs a comment to explain that it's not evaluated. - [x] Actually, when I tried to run it I got the following error:
manual_classes <- labelFeatsManual(feat_peak_info, ms1_data = msdata$MS1, selection = "Labeled", existing_labels = lasso_classes)
Error in labelFeatsManual(feat_peak_info, ms1_data = msdata$MS1, selection = "Labeled", :
unused arguments (selection = "Labeled", existing_labels = lasso_classes)
Looks like the function arguments changed but the vignette wasn't updated to reflect the change.
And something I should've picked up earlier:
- [x] There's need for
here()in the vignette. Just do e.g.,knitr::include_graphics("intro_manual_ss.png")(see https://r-pkgs.org/vignettes.html#filepaths). You can then remove here as a dependency.
Received a valid push on git.bioconductor.org; starting a build for commit id: defbc6a84a8ae2f2de0670ad6f699d2a969f79e8
@PeteHaitch Thanks for the quick response during your conference! Glad the new shiny app worked well on your end too. I've fixed the notes you added above and implemented a few new bug fixes in this version as I've continued using it for my own research purposes.
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): squallms_0.99.6.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/squallms to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Thank you for making the requested changes, @wkumler.
I'm now happy to accept squallms into Bioconductor. Congratulations and thank you for your contribution!
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.
Hooray! Thanks again for your thoughtful guidance and patience throughout this process.
You're very welcome! Thanks for engaging constructively with the review process.
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/wkumler.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("squallms"). The package 'landing page' will be created at
https://bioconductor.org/packages/squallms
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.