search

Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

CluMSIDdata #933

Closed tdepke closed 5 years ago

tdepke commented 6 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 help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

bioc-issue-bot commented 6 years ago

Hi @tdepke

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: CluMSIDdata
Type: Package
Title: Data for the CluMSID package
Version: 0.99.0
Authors@R: person("Tobias", "Depke", email = "tobias.depke@helmholtz-hzi.de", 
    role = c("aut", "cre"))
Maintainer: Tobias Depke <tobias.depke@helmholtz-hzi.de>
Description: This package contains various LC-MS/MS and GC-MS data
    that is used in vignettes and examples in the CluMSID package.
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
Depends:
    R (>= 3.5)
biocViews: ExperimentData, MassSpectrometryData
bioc-issue-bot commented 5 years ago

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

IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your repository will NOT trigger a new build.

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

5774fef Bioconductor-related version bump

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

63f76e3 updated R version dependency

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

65df478 updated R version dependency to 3.6

bioc-issue-bot commented 5 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.

tdepke commented 5 years ago

AdditionalPackage: https://github.com/tdepke/CluMSID

bioc-issue-bot commented 5 years ago

Hi @tdepke,

Starting build on additional package https://github.com/tdepke/CluMSID.

IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your additional package repository will NOT trigger a new build.

The DESCRIPTION file of this additional package is:

Package: CluMSID
Type: Package
Title: Clustering of MS2 Spectra for Metabolite Identification
Version: 0.99.0
Authors@R: c(person("Tobias", "Depke", email = "tobias.depke@helmholtz-hzi.de", 
role = c("aut", "cre")),
person("Raimo", "Franke", email = "raimo.franke@helmholtz-hzi.de",
role = "ctb"),
person("Mark", "Broenstrup", email = "mark.broenstrup@helmholtz-hzi.de",
role = "ths"))
Maintainer: Tobias Depke <tobias.depke@helmholtz-hzi.de>
Description: CluMSID is a tool that aids the identification of features 
in untargeted LC-MS/MS analysis by the use of MS2 spectra similarity 
and unsupervised statistical methods. It offers functions for a complete 
and customisable workflow from raw data to visualisations and is
interfaceable with the xmcs family of preprocessing packages.
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
Depends:
R (>= 3.6)
biocViews: Metabolomics, Preprocessing, Clustering
Imports:
mzR,
S4Vectors,
dbscan,
RColorBrewer,
ape,
network,
GGally,
ggplot2,
plotly,
methods,
stats,
sna,
grDevices,
graphics,
Biobase,
gplots
RoxygenNote: 6.1.1
Suggests: knitr,
rmarkdown,
testthat,
dplyr,
readr,
stringr,
magrittr,
MSnbase,
CluMSIDdata,
metaMS,
metaMSdata,
xcms
VignetteBuilder: knitr
bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

740371d removed invalid character from GC_post.csv

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

e4130b9 deleted .Rhistory 6120f22 changed import for "show" 3d10899 version bump to 0.99.1

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

690d40b started avoiding class()== b3b7e0e version bump to 0.99.2

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

381a5d5 added missing dependency for is()

bioc-issue-bot commented 5 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, TIMEOUT, 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

cbffa0e reduced size of vignettes

bioc-issue-bot commented 5 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, TIMEOUT". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

1b87aa3 added some more data for CluMSID MTBLS vignette

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

0ed5943 modified data import for CluMSID MTBLS vignette

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

7cbdc86 bugfix in MTBLS vignette

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

f780f2c further reduced size of vignettes

bioc-issue-bot commented 5 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.

LiNk-NY commented 5 years ago

Hi Tobias, @tdepke

Thank you for your submission. Please see the review below. If you have any questions, feel free to post them here.

Best regards, Marcel

CluMSID #933

DESCRIPTION

NAMESPACE

R

Minor:

vignettes

LiNk-NY commented 5 years ago

Hi Tobias, @tdepke Please respond to the review or the issue will be closed due to inactivity. Best regards, Marcel

tdepke commented 5 years ago

Hi Marcel, @LiNk-NY thank you for your review! I'm sorry for being so slow, I will revise the package and respond to your review in the next days. Best regards, Tobias

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

8dc57d2 Bioconductor review I: DESCRIPTION and NAMESPACE

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

e7f2d04 Bioconductor review II: R, part one

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

5f283cb Bioconductor review II: R, part two (replaced nest...

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

525f2a0 fixed bug in new distanceMatrix()

bioc-issue-bot commented 5 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.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

d83a726 Bioconductor review II: R, part three

bioc-issue-bot commented 5 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.

tdepke commented 5 years ago

Hi Marcel, @LiNk-NY thanks again for the review! Please find my point-by-point response below. I look forward to receiving your feedback. Best regards, Tobias

CluMSID #933

DESCRIPTION

  • Please add BugReports and URL fields (if available)

Done.

NAMESPACE

  • Stick to either camelCase or snake_case. Try to avoid mixing both types.

I changed all names to camelCase.

  • Avoid using a CluMSID_ prefix. It is not necessary and can be confusing.

All CluMSID_ prefixes have been removed.

  • Ordinary functions should start with lower case letters for instance, Featurelist becomes featureList unless it is a class.

Done.

  • You have more packages under the Imports field than documented in the NAMESPACE. They should have some level of concordance.

I changed that.

R

  • Why save an Rdata file in extdata than create it in the examples (for access_id)? The extdata should be raw data for a read* type of function.

I did that because it would take too long to generate the respective objects from raw data for each example. I agree that it is not ideal to save the Rdata files in extdata. Would it be okay to move these files to \data? That would make them accessible to the user which is neither necessary nor intended. Do you have a suggestion how to handle these files?

  • Use is() to test for class memberships rather than class(x) %in% ...

Done.

  • It would be better to use generics that are already established in other packages or create your own generic and method functions for slot accession.

I generally agree which is why I added methods for precursorMz, rtime, intensity, mz and peaksCount from MSnbase that can be used to access information from MS2spectrum objects but I need additional accessor functions for neutral_losses, annotation, id and spectrum (as a whole, not m/z and intensities separately). For the sake of consisteny, I also kept the accessPrecursor and accessRT functions.

  • Provide all the options in the type argument as a vector, i.e., c("spectrum", "neutral_losses") so that it is visible to the user and then use match.arg to take the first one within the function.

Done.

  • It is better to not assume that the user will input a MS2spectrum class and check for class membership within the getSimilarities function.

Done.

  • Avoid using cat and use message instead. Use cat for show methods only.

Done.

  • Although R has improved copy and append methods, these should be avoided when possible. It would be simpler to use vapply for looping along featlist in findFragment and other functions. You can even make this subsetter check into a helper function for reuse in other functions incl. findNL and findFragment.

Done.

  • Replace if else logic in getSpectrum using a switch() call that returns just the unevaluated condition i.e., what %in% m, abs(what - m) <= mz.tol, abs(what - m) <= rt.tol. You would have to quote the expression and evaluate later.

Done.

  • Avoid use of ;. You can just do id <- mz <- rt <- c() but this shouldn't be necessary if you're using lapply (in Featurelist).

I changed the respective line to id <- mz <- rt <- c() as you suggested.

  • Use as(...) coercion instead of the convertSpectrum function.

Conversion can now be done using either as(object, "MS2spectrum") or as.MS2spectrum(object).

  • Some code can be reduced in extractMS2spectra by using a variable when recalibrate_precursor is TRUE

I am not quite sure if that is what you meant but I reduced the respective code from 39 to 16 lines by using a function for the precursor checking.

  • Avoid nested for loops where possible.

I replaced the nested for-loop in distanceMatrix with a combination of utils::combn and vapply. This did not increase speed, though.

  • Why not let the user specify the column name for mz and rt in extractPseudospectra? This way you don't have to check whether it is in the data.frame.

The idea was to enable direct and easy input of files/objects generated with xcms/CAMERA but they have this naming inconsistency. Therefore, I decided to check for both possibilities rather than letting the user specify and setting one of the possibilities as default.

Minor:

  • FUN.VALUE in splitPolarities should just be logical(1L) it doesn't have to be a conditional expression.

I changed that.

  • Open up specplot (with ...) for users to access additional arguments in plot. This makes plotting more customizable.

Done.

  • No need to test for TRUE within an if function: if (recalibrate_precursor), if you're unsure if the result will be logical, you can use isTRUE(recal...)

This has been changed in all respective functions.

  • Avoid 2:x sequence generation in the event that x = 0. Use seq_along(x)[-1]

Done.

vignettes

  • Looks good.
  • Consider using BiocStyle.

I would like to use BiocStyle but the problem is that BiocStyle::html_document leads to rather large file sizes, comparable to rmarkdown::html_document, whereas rmarkdown::html_vignette produces much smaller files. As CluMSID is already quite big, I had to reduce the file sizes of the vignettes and thus decided to use rmarkdown::html_vignette. I am open to suggestions regarding this issue.

  • Remember to change your download instructions to BiocManager::install() once it is accepted.

I will.

LiNk-NY commented 5 years ago

Hi Tobias, @tdepke Thanks for making those changes. It's looking good! Thank you for submitting to Bioconductor! Best regards, Marcel

bioc-issue-bot commented 5 years ago

Your package has been accepted. It will be added to the Bioconductor Git repository and nightly builds. Additional information will be posed to this issue in the next several days.

Thank you for contributing to Bioconductor!

tdepke commented 5 years ago

Hi Marcel, @LiNk-NY Thank you very much for reviewing my package! Best regards, Tobias

mtmorgan commented 5 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/tdepke.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("CluMSIDdata"). The package 'landing page' will be created at

https://bioconductor.org/packages/CluMSIDdata

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.

mtmorgan commented 5 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/tdepke.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("CluMSID"). The package 'landing page' will be created at

https://bioconductor.org/packages/CluMSID

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.