SDAMS

[Stat-Li]

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

• Repository: https://github.com/Stat-Li/SDAMS

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 that a minimum requirement for package acceptance is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS. Passing these checks does not result in automatic acceptance. The package will then undergo a formal review and recommendations for acceptance regarding other Bioconductor standards will be addressed.

• [x] My package addresses statistical or bioinformatic issues related to the analysis and comprehension of high throughput genomic data.

• [x] I am committed to the long-term maintenance of my package. This includes monitoring the support site for issues that users may have, subscribing to the bioc-devel mailing list to stay aware of developments in the Bioconductor community, responding promptly to requests for updates from the Core team in response to changes in R or underlying software.

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

[bioc-issue-bot]

Hi @Stat-Li

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: SDAMS
Type: Package
Title: Differential Abundant Analysis for Metabolomics and Proteomics Data
Version: 0.99.0
Date: 2018-1-30
Author: Yuntong Li <yuntong.li@uky.edu>, Chi Wang <chi.wang@uky.edu>, 
     Li Chen <lichenuky@uky.edu>
Maintainer: Yuntong Li <yuntong.li@uky.edu>
Depends: R(>= 3.4.0)
Suggests: testthat
Imports: trust, qvalue, Biobase, methods
Description: This Package utilizes a Semi-parametric Differential Abundance 
     analysis (SDA) method for metabolomics and proteomics data from mass 
     spectrometry. SDA is able to robustly handle non-normally distributed data 
     and provides a clear quantification of the effect size.
License: GPL
Encoding: UTF-8
LazyData: true
biocViews: DifferentialExpression, Metabolomics, Proteomics, MassSpectrometry

[bioc-issue-bot]

Your package has been approved for building. Your package is now submitted to our queue.

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]

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

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

87d4c5d 2nd commit 98da497 2nd commit

[bioc-issue-bot]

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]

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

f05add0 2nd commit 4af06c4 3rd commit

[bioc-issue-bot]

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]

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

77018b6 4th commit

[bioc-issue-bot]

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]

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

a1490d1 5th commit

[bioc-issue-bot]

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: "ABNORMAL". 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]

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

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

1906e8b Update DESCRIPTION

[bioc-issue-bot]

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.

[hpages]

Hi @Stat-Li ,

Thanks for submitting the SDAMS package. The SDAMS package introduces a new class, the MSset class, for storing "data from mass spectrometry". Is that necessary? It's important to keep in mind that one of the strengths of the Bioconductor project is to promote the use of core containers that are well designed, well tested, and highly re-usable. New containers should be introduced only when necessary i.e. only when the existing containers are not suitable for the kind of data and/or analysis that one is aiming at. More precisely, here are the reasons why I'm not convinced that the MSset class is needed:

• The man page for the MSset class says:

  The ‘MSset’ is a s4 class used to store data from mass spectrometry. This class a
  subclass of ‘ExpressionSet’, with one more slot: group.

  However I don't see the 'group' slot:

  > showClass("MSset")
  Class "MSset" [package "SDAMS"]
  
  Slots:
  
  Name:      experimentData          assayData          phenoData
  Class:              MIAME          AssayData AnnotatedDataFrame
  
  Name:         featureData         annotation       protocolData
  Class: AnnotatedDataFrame          character AnnotatedDataFrame
  
  Name:   .__classVersion__
  Class:           Versions
  
  Extends: 
  Class "ExpressionSet", directly
  Class "eSet", by class "ExpressionSet", distance 2
  Class "VersionedBiobase", by class "ExpressionSet", distance 3
  Class "Versioned", by class "ExpressionSet", distance 4

  So an important question is: why do you need to define a new class that extends ExpressionSet when you could just use an ExpressionSet object to store your data?

• Furthermore, the eSet/ExpressionSet classes (defined in the Biobase package) are considered a little obsolete these days and superseeded by the more recent SummarizedExperiment class defined in the SummarizedExperiment package. I would highly recommend that you get familiarized with SummarizedExperiment and RangedSummarizedExperiment objects and that you use these objects to store your data. It could also be that Bioconductor already has dedicated containers for storing data from mass spectometry so I would suggest that you check what Bioconductor has to offer in the mass spectometry department:

  https://bioconductor.org/packages/release/BiocViews.html#___MassSpectrometry

  Don't hesitate to ask on the bioc-devel mailing list if you need advice about this. Many of the authors/maintainers of the core packages for proteomics (ProtGenerics, mzR, xcms, MSnID, MSnbase, etc...) are monitoring bioc-devel and will provide valuable advice.

• One small thing unrelated to the discussion about the MSset class: Please use consistent naming style e.g. createMSsetFromCSV instead of createMSsetfromCSV.

Thanks, H.

[bioc-issue-bot]

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

64be166 class issue

[bioc-issue-bot]

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

9b30229 Update DESCRIPTION

[bioc-issue-bot]

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]

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.

[Stat-Li]

Dear @hpages ,

Thank you for your feedback. The SummarizedExperiment object is suitable for our data structure. I have made some changes based on your suggestions.

[hpages]

Hi @Stat-Li ,

Thanks for the update. I took a closer look at the SDAMS package and found several issues. One major issue is that the SDA() function is currently a black box because of lack of documentation. Please see the details below. Don't hesitate to ask me or on the bioc-devel mailing list if you have questions or concerns about this. Best, H.

1. Remove the SDAMS.tex, SDAMS.R, and SDAMS.pdf files from the SDAMS/vignettes/ folder. These files don't belong to the package source tree and should not be included in it.

2. The package source tree is polluted with other files e.g. the SDAMS/R/, SDAMS/tests/testthat/, and SDAMS/vignettes/ folders contain .Rhistory files. Make sure to clean it by removing all the files that don't belong to it.

3. Where is the data in CSV files ProstateFeature.csv and ProstateGroup.csv coming from? What kind of data is it? How was it generated? How were the CSV files made? You need to provide a lot more details. The purpose of submitting the SDAMS package to Bioconductor is of course that people will be able to use it on their own data. This process needs to be documented.

4. Please provide more detailed information about the data in the exampleSumExpset data set. Again please explain what kind of data it contains, how was the data generated, how was the object created, what criteria was used for the grouping, etc... All the man page is saying is:

   This example data is created based on a real proteomics data with 560 features for
   202 experimental subjects.

   which is too mimimalist. The vignette provides a reference to detailed information regarding the data set, which is good, but the man page (which is the primary documentation) should also provide that reference.

5. Several functions and objects contain set in their name (createSEsetFromCSV, createSEsetFromEnvir, exampleSEset, exampleSEset2, etc...). This looks like a leftover from when MSset was used instead of SummarizedExperiment to store the data. Now that you've switched to SummarizedExperiment, the presence of set in these names is surprising, confusing and misleading.

6. The exampleSumExpset data set is another misleading name that looks like a leftover from when the data was stored in an MSset object (which was deriving from ExpressionSet).

7. Don't use "SummarizedExperiment constructor" as the title of the man pages for createSEsetFromCSV() and createSEsetFromEnvir(). Even though these functions return a SummarizedExperiment object, they should not be confused with the SummarizedExperiment() constructor function defined in the SummarizedExperiment package. In other words, not because a function returns an object of class A means it should be called a constructor.

8. The name and description of the createSEsetFromEnvir() function are misleading. This function doesn't create a SummarizedExperiment object "from R global environment" but from other R objects passed to it (a matrix and a data.frame in that case). These objects could be defined anywhere, not just in the R global environment. Where the objects/values passed to the arguments of a function are defined is irrelevant as far as the name of the function is concerned (i.e. the name of the function should not try to capture that).

9. The body of the createSEsetFromCSV() function contains exactly the same 30 lines of code as the body of the createSEsetFromEnvir() function, except for the 4 additional lines at the beginning. An important and powerful principle of good software design is the DRY principle (Don't Repeat Yourself): https://en.wikipedia.org/wiki/Don%27t_repeat_yourself Please apply this principle to your code. In the case of createSEsetFromCSV(), this means that the function should call createSEsetFromEnvir() internally.

10. Please describe in much greater details what the SDA() function does, how it works, what the components of the list it returns are, how to interpret them, etc... in the man page of the function. This is the central function of your package and the only reason why people would use the SDAMS package.

11. The Data Analysis section of the vignette couldn't be more minimalist. It basically shows a call to SDA() with a 4-line preamble that doesn't say much and is followed by several calls to head() to show the first values of 6 of the 9 components of the returned list. With no more explanations about what SDA() does, how it works(), why it implements a useful and reliable algorithm, why this algorithm performs well, you're providing a blackbox that the user is asked to trust. Conscious data analysts want clarity and transparency so they can make informed choices.

12. Typos in man pages and vignette:

    • should be "model", not "mdoel"

    • should be "point", not "ponit"

    • should be "csv files", not "csv.files"

13. Use file.path(...) instead of paste(..., sep="/") to construct file paths in a platform-independent way.

14. Coding style:

    • Always use the left arrow operator (<-) for assignment, with a before and after it.

    • Always put a space after a comma (,).

    So for example:

    path1 <- file.path(directory1, "ProstateFeature.csv")

    instead of

    path1<-paste(directory1,"ProstateFeature.csv",sep="/")

[bioc-issue-bot]

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

8cba633 more explanation

[bioc-issue-bot]

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

b27fa4b Update DESCRIPTION

[bioc-issue-bot]

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]

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]

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

405ef14 Update DESCRIPTION

[bioc-issue-bot]

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.

[Stat-Li]

Dear @hpages ,

Thank you for your comments. We have updated our package:

1. SDAMS.tex, SDAMS.R and SDAMS.pdf in the vignettes/ folder removed;
2. .Rhistory and .DS_store files has been removed from all the folders;
3. We provide two kinds of format to store the example data, .rda and csv files. I create a new exampleData.RD file to explain those data including descriptions and reference;
4. Solved in Issue 3;
5. The 'set' was removed from the function and example data names;
6. 'set' is also removed;
7. 'dataInput.RD' is created to explain both createSEFromCSV and createSEFromMatrix;
8. change createSEFromEnvir to createSEFromMatrix;
9. In function createSEFromCSV, we just call createSEFromMatrix;
10. We provide more details in SDA.RD, the model, parameters and their interpretations;
11. In vignettes, we give more information regarding the example data. For data analysis, the explanation is provided for each returned component and we add a new theory section to explain the theory behind the SDAMS;
12. Typos corrected;
13. Switched to file.path() for file location;
14. Coding style changed.

Thanks again!

[Stat-Li]

Hi @hpages ,

Any further comments? We hope we could catch up with the BioC 3.7 release. Thank you for your time reviewing this package.

Best, Yuntong

[Stat-Li]

Dear @hpages ,

Since I didn't hear any further comments from you, I just want to make sure the modification I made is OK. Looking forward to hear your feedback. Thank you.

Yuntong

[hpages]

Hi Yuntong,

Thanks for your work on SDAMS. The improvements are great and everything looks fine now. I'm accepting the package.

Cheers, H.

[bioc-issue-bot]

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!

[mtmorgan]

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/Stat-Li.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 biocLite(\"SDAMS\"). The package 'landing page' will be created at

https://bioconductor.org/packages/SDAMS

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.