squallms

[wkumler]

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]'

• I understand that by submitting my package to Bioconductor,
  the package source and all review commentary are visible to the
  general public.

• I have read the Bioconductor Package Submission
  instructions. My package is consistent with the Bioconductor
  Package Guidelines.

• I understand Bioconductor Package Naming Policy and acknowledge
  Bioconductor may retain use of package name.

• 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.

• My package addresses statistical or bioinformatic issues related
  to the analysis and comprehension of mass-spectrometry data.

• 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 Bioconductor code of conduct and
  agree to abide by it.

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

• The 'devel' branch for new packages and features.
• The stable 'release' branch, made available every six
  months, for bug fixes.
• 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.

[bioc-issue-bot]

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 = "wkumler@uw.edu",
         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

[lshep]

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.

[bioc-issue-bot]

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

[bioc-issue-bot]

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
git@git.bioconductor.org:packages/squallms to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: 9115a56d52d5371613242f22093329a3208a7f5e

[bioc-issue-bot]

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
git@git.bioconductor.org:packages/squallms to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: f3c495c697f29acb0665a6e1ce8a73b11cec492e

[bioc-issue-bot]

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
git@git.bioconductor.org:packages/squallms to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: d62e83477270529f4f08b48b98f696875cc544e4

[bioc-issue-bot]

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
git@git.bioconductor.org:packages/squallms to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

A reviewer has been assigned to your package for an indepth review.
Please respond accordingly to any further comments from the reviewer.

[PeteHaitch]

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

[wkumler]

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

[PeteHaitch]

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 same lasso_classes and manual_classes variables 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 verbosity argument of extractChromMetrics() 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 ?logModelFeatProb should 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 (see covr::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/CITATION file (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.

[wkumler]

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.

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: acd422f235dbcc938ad7285d587197f6966ca19f

[bioc-issue-bot]

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
git@git.bioconductor.org:packages/squallms to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.

[PeteHaitch]

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

• Check formatting of figure captions (e.g., there's a literal #fig:plot metrics in rendered vignette). In this case, chunk labels should not contain spaces, so you'll want to use something like plot-metrics instead of plot metrics (see https://yihui.org/knitr/options/#chunk-options).
• In the vignette, please note with a comment in the corresponding code chunk or text nearby any eval=FALSE chunks, e.g., the manual_classes <- labelFeatsManual(feat_peak_info, ms1_data = msdata$MS1, selection = "Labeled", existing_labels = lasso_classes)
  • Furthermore, for this specific chunk, if the user runs it then they will have a different manual_classes vector for subsequent steps, which might not be ideal because subsequent steps will give different results, so perhaps note that?

Recommended

• 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).
• 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?