CAMTHC - Githubissues

Lululuella commented 6 years ago

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

Repository: https://github.com/Lululuella/CAMTHC

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

Hi @Lululuella

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: CAMTHC
Type: Package
Title: Convex Analysis of Mixtures for Tissue Heterogeneity Characterization
Version: 0.99.0
Author: Lulu Chen <luluchen@vt.edu>
Maintainer: Lulu Chen <luluchen@vt.edu>
biocViews: Software, CellBiology, GeneExpression
Description: An R package for tissue heterogeneity characterization by convex 
    analysis of mixtures (CAM). It provides basic functions to perform 
    unsupervised deconvolution on mixture expression profiles by CAM and some 
    auxiliary functions to help understand the subpopulation-specific results. 
    It also implements functions to perform supervised deconlution based on 
    prior knowledge of molecular markers, S matrix or A matrix. Combining 
    molecular markers from CAM and from prior knowledge can achieve 
    semi-suprvised deconvolution of mixtures.
License: GPL-2
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.0.1
Depends: R (>= 3.5)
Suggests: 
    knitr,
    rmarkdown,
    BiocStyle,
    testthat,
    GEOquery,
    rgl
VignetteBuilder: knitr
Imports: 
    rJava,
    BiocParallel,
    stats,
    corpcor,
    geometry,
    NMF,
    DMwR,
    pcaPP,
    apcluster,
    graphics
SystemRequirements: Java (>= 1.8)

bioc-issue-bot commented 6 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 6 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 6 years ago

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

ab4d4b8 reduce vignette size; remove apclusterK link error

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

lshep commented 6 years ago

Thank you for your interest in Bioconductor. Please see the following comments

README:

[ ] typos in readme: ie. comoputational

[ ] update to have Bioconductor installation instructions

## try http:// if https:// URLs are not supported
source("https://bioconductor.org/biocLite.R")
biocLite("CAMTHC")

General:

[ ] Please git rm CAMTHC.Rproj If you would like to keep this file locally in the future please add to .gitignore

Vignette/R code:

[ ] Your main function should also work with standard Bioconductor objects for gene expression data, proteomics data and DNA methylation data. - Ideally take the Bioconductor object as input and coerace to needed format for you function - this could be in your main function or provide wrapper functions for the Bioconductor object
[ ] You also define many S3 internal classes that become values in the object you return from your main function (MDLObj, CAMPrepObj, CAMASObj, CAMMGObj) . These should be converted to S4 classes and provide accessor methods for getting and setting values. The output of your main CAM function should also be converted into an S4 object with associated getter accessor methods rather than a generic list with obscure accessing through $ and [. Not only would this clean up and better define the output, but the accessor methods would get rid of messy and undescriptive code like
```
Aest <- rCAM$ASestResult$'3'$Aest
Sest <- rCAM$ASestResult$'3'$Sest
# or 
m <- which(names(rCAM$ASestResult) == '3')
Aest <- rCAM$ASestResult[[m]]$Aest
Sest <- rCAM$ASestResult[[m]]$Sest
```
could have an accessor method of Aest and Sest
[ ] In your vignette, In your lapply's - 1:n should also use use seq_len()
i.e 1:3 should be seq_len(3)
[ ] Case Study section is Excellent! We like showing it can be used with Bioconductor object - However - instead of having the user perform these can you create a wrapper around the object to be used with the main function.

Please address these issue. When ready for a re-review please perform a version bump to kick off a new build and comment here with how the code has been updated.

Cheers Lori

lshep commented 6 years ago

Hello. Just wanted to check it to see if you had any questions or when we might expect to see updates. If there is no response within two weeks we will close the issue for inactivity and you can reopen the issue when you can work on updates.

Lululuella commented 6 years ago

Hello,

Sorry for being late. Do I still have time to update the software this week? Thanks a lot!

Best, Lulu

On Tue, May 8, 2018 at 12:18 PM, lshep notifications@github.com wrote:

Hello. Just wanted to check it to see if you had any questions or when we might expect to see updates. If there is no response within two weeks we will close the issue for inactivity and you can reopen the issue when you can work on updates.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/726#issuecomment-387458856, or mute the thread https://github.com/notifications/unsubscribe-auth/AdpXAW3t8ZKO9QT8zzcLmn1aWLAUi8-0ks5twcVYgaJpZM4TVVcO .

lshep commented 6 years ago

Of course! We just want to make sure that there is forward moving progress. When you are ready for a re-review please make sure there is a version bump in the DESCRIPTION file to kick off a new build and please comment back here with what has been updated.
Cheers!

Lululuella commented 6 years ago

Hello, do I need to update my current R and bioconductor version?

lshep commented 6 years ago

I'm unsure what you mean by update - Your R version in your description is R >= 3.5 which is the version of R we are currently using. If you want to test locally - the versions we are using to test your package are Bioc 3.8 and R 3.5.0

bioc-issue-bot commented 6 years ago

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

7d43f9b rm Rproj file 689656f 0.99.2 version

bioc-issue-bot commented 6 years ago

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

85d9bcf 0.99.3

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

Lululuella commented 6 years ago

Hello, I update my package following your suggestion. Objects returned from main functions are converted to S4 classes with some useful accessor methods. Input expression data could be SummarizedExperiment and ExpressionSet object. The second Case Study used ExpressionSet type as input. Please review it again. Thanks a lot!

lshep commented 6 years ago

Hi. Looking much better thank you for making the switch to S4 - some minor clean up:

NEWS

[ ] NOTE: Consider adding a NEWS file, so your package news will be included in Bioconductor release announcements. If the NEWS file is in inst please remove the md extension (but can still be written in markdown)
[ ] Formating. Please change the title of the section from # CAMTHC 0.99.0 to Changes in verison 0.99.0

DESCRIPTION

[ ] Remove LazyData: true. This rarely proves to be a good thing. In our experience it only slows down the loading of packages with large data

Please make the above changes and do another version bump for a new build.

bioc-issue-bot commented 6 years ago

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

74f6c7e verison 0.99.4 Remove LazyData:true. fix bugs ...

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

Lululuella commented 6 years ago

Hi, I have a new warning: "Remove set.seed usage in R code". I use the seed to reproduce the same result. Now it is not encouraged? Do you have any suggestion?

lshep commented 6 years ago

It is encouraged but normally we encourage its use outside of R code - for example set.seed in the examples or vignette before running an R function - I looked at your code at it seems you set the seed by argument, and if the seed is not specified than it is not used - is that correct?

Lululuella commented 6 years ago

Thanks! I will remove seed argument and set it outside of functions.

bioc-issue-bot commented 6 years ago

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

32a6bc2 Verison 0.99.5 * Remove Remove set.seed usage with...

bioc-issue-bot commented 6 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 6 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!

mtmorgan commented 6 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/Lululuella.keys is not empty), then no further steps are required. Otherwise, do the following:

See further instructions at

https://bioconductor.org/developers/how-to/git/

for working with this repository. See especially

https://bioconductor.org/developers/how-to/git/new-package-workflow/ https://bioconductor.org/developers/how-to/git/sync-existing-repositories/

to keep your GitHub and Bioconductor repositories in sync.

Your package will be included in the next nigthly 'devel' build (check-out from git at about 6 pm Eastern; build completion around 2pm Eastern the next day) at

https://bioconductor.org/checkResults/

(Builds sometimes fail, so ensure that the date stamps on the main landing page are consistent with the addition of your package). Once the package builds successfully, you package will be available for download in the 'Devel' version of Bioconductor using biocLite(\"CAMTHC\"). The package 'landing page' will be created at

https://bioconductor.org/packages/CAMTHC

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.

Lululuella commented 6 years ago

Hi, my package's "devel" builds failed in windows: .onLoad failed in loadNamespace() for 'rJava'. This problem did not exist in previous check. Could anyone help me to fix this problem?

lshep commented 6 years ago

Please post to the bioc-devel@r-project.org for questions on the daily builders and how to resolve ERRORs - Cheers

Bioconductor / Contributions

CAMTHC #726