Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

MsBackendRawFileReader Public #2339

Closed cpanse closed 2 years ago

cpanse commented 2 years ago

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

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

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

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 commented 2 years ago

Hi @cpanse

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: MsBackendRawFileReader
Type: Package
Title: Bridging Spectra and Thermo Fisher Scientific raw files
Version: 0.99.0
Authors@R: c(person(given = "Christian",
    family = "Panse", email = "cp@fgcz.ethz.ch", role = c("aut", "cre"),
    comment = c(ORCID = "0000-0003-1975-3064")),
    person(given = "Tobias", family = "Kockmann",
      email = "Tobias.Kockmann@fgcz.ethz.ch", role = "aut", 
    comment = c(ORCID = "0000-0002-1847-885X")))
Depends: R (>= 4.1), methods, Spectra (>= 1.2)
Imports:
    MsCoreUtils,
    S4Vectors,
    IRanges,
    rawrr (>= 1.1.10),
    utils,
    BiocParallel
Suggests:
    BiocStyle (>= 2.5),
    ExperimentHub,
    knitr,
    lattice,
    protViz (>= 0.6),
    rmarkdown,
    tartare (>= 1.5),
    testthat
Description: implements a MsBackend for the Spectra package using
  Thermo Fisher Scientific's NewRawFileReader .Net libraries.
  The package is generalizing the functionality introduced by the
  rawrr package (Kockmann, 2021 <doi:10.1021/acs.jproteome.0c00866 >).
  Methods defined in this package are supposed to extend the Spectra
  Bioconductor package. 
URL: https://github.com/fgcz/MsBackendRawFileReader
BugReports: https://github.com/fgcz/MsBackendRawFileReader/issues
Encoding: UTF-8
NeedsCompilation: yes
biocViews: MassSpectrometry, Proteomics, Metabolomics
RoxygenNote: 7.1.2
License: GPL-3
VignetteBuilder: knitr
Collate: 
    'hidden_aliases.R'
    'AllGenerics.R'
    'MsBackendRawFileReader-functions.R'
    'MsBackendRawFileReader.R'
    'zzz.R'
bioc-issue-bot commented 2 years ago

A reviewer has been assigned to your package. Learn what to expect during the review process.

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. It is required to push a version bump to git.bioconductor.org to trigger a new build.

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 commented 2 years ago

Dear Package contributor,

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

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

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

Please see the build report for more details. This link will be 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/MsBackendRawFileReader to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

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

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

Please see the build report for more details. This link will be 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/MsBackendRawFileReader to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details. This link will be 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/MsBackendRawFileReader to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

lshep commented 2 years ago

I'm trying to build, install, and check locally and I'm running into the following ERROR:

(base) shepherd@jbcj433:~/PkgReviews$ R CMD build MsBackendRawFileReader 
* checking for file 'MsBackendRawFileReader/DESCRIPTION' ... OK
* preparing 'MsBackendRawFileReader':
* checking DESCRIPTION meta-information ... OK
* installing the package to build vignettes
* creating vignettes ... ERROR
--- re-building ‘MsBackendRawFileReader.Rmd’ using rmarkdown
trying URL 'https://github.com/thermofisherlsms/ThermoRawFileParser/raw/master/packages/ThermoFisher.CommonCore.RawFileReader.4.0.26/lib//ThermoFisher.CommonCore.Data.dll'
Content type 'application/octet-stream' length 360448 bytes (352 KB)
==================================================
downloaded 352 KB

trying URL 'https://github.com/thermofisherlsms/ThermoRawFileParser/raw/master/packages/ThermoFisher.CommonCore.RawFileReader.4.0.26/lib//ThermoFisher.CommonCore.MassPrecisionEstimator.dll'
Content type 'application/octet-stream' length 11264 bytes (11 KB)
==================================================
downloaded 11 KB

trying URL 'https://github.com/thermofisherlsms/ThermoRawFileParser/raw/master/packages/ThermoFisher.CommonCore.RawFileReader.4.0.26/lib//ThermoFisher.CommonCore.RawFileReader.dll'
Content type 'application/octet-stream' length 620544 bytes (606 KB)
==================================================
downloaded 606 KB

trying URL 'http://fgcz-ms.uzh.ch/~cpanse/rawrr/rawrr.1.1.12.exe'
Content type 'application/x-msdos-program' length 26624 bytes (26 KB)
==================================================
downloaded 26 KB

snapshotDate(): 2021-10-06
see ?tartare and browseVignettes('tartare') for documentation
loading from cache
see ?tartare and browseVignettes('tartare') for documentation
loading from cache
see ?tartare and browseVignettes('tartare') for documentation
loading from cache

Unhandled Exception:
System.IO.FileNotFoundException: Could not load file or assembly or one of its dependencies.
File name: 'System.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089'

Unhandled Exception:
System.IO.FileNotFoundException: Could not load file or assembly or one of its dependencies.
File name: 'System.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089'

Unhandled Exception:
System.IO.FileNotFoundException: Could not load file or assembly or one of its dependencies.
File name: 'System.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089'
Quitting from lines 94-97 (MsBackendRawFileReader.Rmd) 
Error: processing vignette 'MsBackendRawFileReader.Rmd' failed with diagnostics:
BiocParallel errors
  3 remote errors, element index: 1, 2, 3
  0 unevaluated and other errors
  first remote error: argument is of length zero
--- failed re-building ‘MsBackendRawFileReader.Rmd’

SUMMARY: processing the following file failed:
  ‘MsBackendRawFileReader.Rmd’

Error: Vignette re-building failed.
Execution halted

Please advise

cpanse commented 2 years ago

@lshep I think mono is not installed on your system. http://bioconductor.org/packages/release/bioc/install/rawrr/INSTALL Christian

lshep commented 2 years ago

I did follow the instructions suggested of sudo apt-get install mono-runtime that was given -- if there is more extensive installation for this package it probably should be noted somewhere to check and reference this installation file. After installing all the recommended system installation of the linux section that indeed fixed. I will continue with a review

lshep commented 2 years ago

Thank you for your submission. Please see comments:

inst

NEWS

Vignettes

MsBackendRawFileReader.Rmd

beRaw <- Spectra::backendInitialize(
  MsBackendRawFileReader::MsBackendRawFileReader(),
  files = c(rawfileEH3220, rawfileEH3222, rawfileEH4547))

because the code to generate rawfileEH####* is not shown.

> SF1 <- (beRaw |>  
+    filterScan("ms2 487.2567"))
> 
> SF2 <- beRaw |>  
+    filterScan("ms2 487.25") |>
+    sparceVector() |> 
+    dotProduct(list(mZ=c(1, 2, 3), intensity=c(10, 100, 100)))
Error in dotProduct(sparceVector(filterScan(beRaw, "ms2 487.25")), list(mZ = c(1,  : 
  could not find function "dotProduct"

This is why we recommend running as much code as possible in the vignette for reproducibility.

tartare.Rmd

> vignette("tartare", package="MsBackendRawFileReader")
Warning message:
vignette 'tartare' not found 

ForDevelopers

> vignette("ForDevelopers", package="MsBackendRawFileReader")
Warning message:
vignette 'ForDevelopers' not found 

Rcode

Please address the above issues and comment back here. When ready please remember to do a version bump to trigger a new build and request a re-review.

Cheers,

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

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

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details. This link will be 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/MsBackendRawFileReader to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

cpanse commented 2 years ago

Dear @lshep; Thank you for your review. I solved all issues. Thank you, Christian

0 cp@leda:~/__checkouts/bioc/MsBackendRawFileReader % git log --oneline                                                                                                                                      (git)-[master]-21-10-13|10:43:18
515f939 (HEAD -> master, origin/master, origin/HEAD) Merge
c3ebfdf Bump 0.99.3
904d9d3 Enhance and correct package vignette
89a585f Remove unused commented R code
a6ba078 Remove tartare.Rmd and ForDevelopers.Rmd files
ba3a002 Add specs.R file in inst/scrtips
45caaa2 Add NEWS.rd file

Thank you for your submission. Please see comments:

inst

* [X]  Please provide an `inst/scripts/` directory that contains a file that
  explains how the data in `inst/extdata` was generated. This can be R code,
  sudo-code, or plain text but minimally should include the source information
  for the data.

NEWS

* [X]  (Optional) We highly recommend a NEWS file for keeping track of changes or updates.
  NEWS files are used to create the Bioconductor release announcements.

Vignettes

MsBackendRawFileReader.Rmd

* [X]  Perhaps put the `stopifnot` for required packages in a `suppressMessage()`
  for easier reading.

* [X]  By hiding code in your vignette I am not able to run the code chunk
beRaw <- Spectra::backendInitialize(
  MsBackendRawFileReader::MsBackendRawFileReader(),
  files = c(rawfileEH3220, rawfileEH3222, rawfileEH4547))

because the code to generate rawfileEH####* is not shown.

* [X]  The data retrieve from the hub is then not utilized in the vignette anywhere?
EH3219 <- normalizePath(eh[["EH3219"]])
EH3221 <- normalizePath(eh[["EH3221"]])
EH4547 <- normalizePath(eh[["EH4547"]])
* [X]  Please show all necessary code to reproduce. I also cannot run the code
  `Application example`
> SF1 <- (beRaw |>  
+    filterScan("ms2 487.2567"))
> 
> SF2 <- beRaw |>  
+    filterScan("ms2 487.25") |>
+    sparceVector() |> 
+    dotProduct(list(mZ=c(1, 2, 3), intensity=c(10, 100, 100)))
Error in dotProduct(sparceVector(filterScan(beRaw, "ms2 487.25")), list(mZ = c(1,  : 
  could not find function "dotProduct"

This is why we recommend running as much code as possible in the vignette for reproducibility.

tartare.Rmd

* [X]  Please rename as there is a `tartare.Rmd` in the tartare package.  When
  trying to pull up your vignette if I do `vignette("tartare")` I do not find
  your vignette, I find the tartare package vignette. and when I try to specify
  your package I cannot read the vignette
> vignette("tartare", package="MsBackendRawFileReader")
Warning message:
vignette 'tartare' not found 

ForDevelopers

* [X]  Same issue with ForDevelopers
> vignette("ForDevelopers", package="MsBackendRawFileReader")
Warning message:
vignette 'ForDevelopers' not found 

Rcode

* [X]  Please remove unused commented out code.

Please address the above issues and comment back here. When ready please remember to do a version bump to trigger a new build and request a re-review.

Cheers,

lshep commented 2 years ago

vignette Again -- I'm not sure why you are doing the following as the don't appear to be used in the code for the vignette anywhere?

EH3219 <- normalizePath(eh[["EH3219"]])
EH3221 <- normalizePath(eh[["EH3221"]])

And the following is done twice which is unnecessary?

EH4547 <- normalizePath(eh[["EH4547"]])

I see EH3220 , EH3222 and EH4547 uses with files = c(rawfileEH3220, rawfileEH3222, rawfileEH4547))

Can this be cleaned up?

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

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

On one or more platforms, the build results were: "ERROR". 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. This link will be 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/MsBackendRawFileReader to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

cpanse commented 2 years ago

vignette Again -- I'm not sure why you are doing the following as the don't appear to be used in the code for the vignette anywhere?

EH3219 <- normalizePath(eh[["EH3219"]])
EH3221 <- normalizePath(eh[["EH3221"]])

And the following is done twice which is unnecessary?

EH4547 <- normalizePath(eh[["EH4547"]])

I see EH3220 , EH3222 and EH4547 uses with files = c(rawfileEH3220, rawfileEH3222, rawfileEH4547))

Can this be cleaned up? @lshep Yes, I fixed that. I might introduce the mzXML files again once I get the mzR package running on my M1 for a MsBackend comparison. C

lshep commented 2 years ago

And that seems like an intermittent error on our part so I'm okay to accept now. thank you

bioc-issue-bot commented 2 years ago

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.

lshep commented 2 years ago

The master branch of your GitHub repository has been added to Bioconductor's git repository.

To use the git.bioconductor.org repository, we need an 'ssh' key to associate with your github user name. If your GitHub account already has ssh public keys (https://github.com/cpanse.keys is not empty), then no further steps are required. Otherwise, do the following:

  1. Add an SSH key to your github account
  2. Submit your SSH key to Bioconductor

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("MsBackendRawFileReader"). The package 'landing page' will be created at

https://bioconductor.org/packages/MsBackendRawFileReader

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.