Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cTRAP #906

Closed
8 tasks done
nuno-agostinho opened this issue Oct 5, 2018 · 35 comments
Closed
8 tasks done

cTRAP #906

nuno-agostinho opened this issue Oct 5, 2018 · 35 comments
Assignees
Labels
3a. accepted will be ingested into Bioconductor daily builder for distribution OK

Comments

@nuno-agostinho
Copy link

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

@bioc-issue-bot
Copy link
Collaborator

Hi @nuno-agostinho

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: cTRAP
Title: Identification of potential transcriptome perturbations
Version: 0.99.1
Authors@R: c(person("Nuno", "Agostinho", 
        email="nunodanielagostinho@gmail.com", role=c("aut", "cre")),
        person("Bernardo", "de Almeida", role="aut"),
        person(c("Nuno", "Luís"), "Barbosa-Morais", role=c("aut", "led")))
Description: Compare differential gene expression results with those from known
    cellular perturbations (such as gene knock-down, overexpression or small 
    molecules) derived from the Connectivity Map. Such analyses allow not only 
    to infer the molecular causes of the observed difference in gene expression 
    but also to identify small molecules that could drive or revert specific 
    transcriptomic alterations.
Depends: R (>= 3.5.0)
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
biocViews: DifferentialExpression, GeneExpression, RNASeq, Transcriptomics,
    Pathways
URL: https://github.com/nuno-agostinho/cTRAP
BugReports: https://github.com/nuno-agostinho/cTRAP/issues
Suggests: testthat
RoxygenNote: 6.1.0
Imports: data.table,
    limma,
    stats,
    fgsea,
    pbapply,
    plyr,
    biomaRt,
    cowplot,
    ggplot2,
    rhdf5,
    R.utils,
    httr,
    methods,
    piano,
    readr,
    utils,
    graphics

@bioc-issue-bot
Copy link
Collaborator

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 bioc-issue-bot added 2. review in progress assign a reviewer and a more thorough review of package code and documentation taking place and removed 1. awaiting moderation labels Oct 5, 2018
@bioc-issue-bot
Copy link
Collaborator

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
Copy link
Collaborator

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

cc87e27 Fix issue in Windows build

@bioc-issue-bot
Copy link
Collaborator

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
Copy link
Collaborator

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

ccbfada Change mode depending on downloading a compressed ...

@bioc-issue-bot
Copy link
Collaborator

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.

@nuno-agostinho
Copy link
Author

Hello @vobencha, I have now fixed all issues with the package, but I am having trouble with running R CMD check within 5 minutes. The problem is that many examples/tests make use of data that is downloaded on demand.

I asked the question at bioc-devel, to which Lori Shepherd replied to check BiocFileCache. Still, wouldn't this make the build fail since it has to download everything the first time to create the cache?

@bioc-issue-bot
Copy link
Collaborator

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

0c232ca Avoid testing function to improve speed when runni...

@bioc-issue-bot
Copy link
Collaborator

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

598419e Avoid testing function to improve speed when runni...

@bioc-issue-bot
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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

5c8c972 Avoid testing function to improve speed when runni...

@bioc-issue-bot
Copy link
Collaborator

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.

@vobencha
Copy link

Hi @nuno-agostinho ,

I'd recommend creating small sample data files for the man pages and vignette. Put them in cTRAP/data/ or cTRAP/inst/extdata. Documentation on what type of file goes where is here:
https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Data-in-packages

Once this is done you can load the file with system.file() or data() - there is no need to create your own function, loadInternalData().

This solution assumes you're downloading data in man pages or vignette, not internally with your package functions. The package functions should avoid downloading on the fly - they will be broken when the user has no internet access.

Valerie

@vobencha
Copy link

We have until Oct 24th if you want this in Bioc 3.8:
https://www.bioconductor.org/developers/release-schedule/

Otherwise the package will only be available in devel (Bioc 3.9) until the Spring 2019 release.

@bioc-issue-bot
Copy link
Collaborator

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

009a160 Use smaller datasets for examples and vignettes

@bioc-issue-bot
Copy link
Collaborator

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
Copy link
Collaborator

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

2a306e0 Fix errors in R CMD check

@bioc-issue-bot
Copy link
Collaborator

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
Copy link
Collaborator

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

afef174 Add more runnable examples

@bioc-issue-bot
Copy link
Collaborator

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
Copy link
Collaborator

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

6257e75 Use smaller example datasets for faster R CMD chec...

@bioc-issue-bot
Copy link
Collaborator

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.

@vobencha
Copy link

vobencha commented Oct 22, 2018

@nuno-agostinho If the package is ready for re-review please articulate what changes were made wrt
#906 (comment)

@nuno-agostinho
Copy link
Author

Hey @vobencha, as per the comparison between commit 5c8c972 and the current master (commit 6257e75), the following changes were made:

  • Data files are now stored in cTRAP/data (for external use) instead of cTRAP/R/sysdata.rda (for internal use)
  • As you suggested, small sample data files were created for the man pages and vignette; they are stored in the cTRAP/data folder and their respective documentation (including code on how to obtain the same sample data) is in the cTRAP/R/cTRAP-package.r file
  • As you suggested, internal data is loaded using the built-in data()function instead of the loadInternalData() function that was created for a redundant purpose
  • Code to download and filter/process data was isolated into new functions (downloadENCODEsamples(), downloadL1000data(), filterL1000metadata() and prepareENCODEgeneExpression()), so users are able to explicitly use the output of those functions as input for downstream functions
  • Changes were made to function examples, test units and vignette to reflect and leverage the new small data samples and functions, among the other changes reported here

Bug fixes, copy-editing and minor changes

  • The file cTRAP/R/l1000-package.r was renamed to cTRAP/R/cTRAP-package.r as the name of the package was changed cTRAP (I apologise for this as I forgot to rename this file before sending this for review)
  • performDifferentialExpression() returned a single mean-aggregate result of differential gene expression statistic for all genes whose ENSEMBL identifier did not match any gene symbol; the mean-aggregate value of such genes is now removed before returning the results
  • Function/variable names with L1000 were uniformed: any letter in a function/variable after L1000 is now lowercase for consistency purposes (e.g. getL1000Conditions() is now getL1000conditions())
  • Expanded the examples for the plotL1000comparison() function

@vobencha
Copy link

vobencha commented Oct 23, 2018

@nuno-agostinho

Excellent. Thanks for making the changes.

Just a few things left:

  • All man pages need running examples if possible; if there are exceptions please explain.
  • Remove vignettes/tutorial.R; only the vignette and graphic files should be in there.
  • Consider renaming the vignette to something more descriptive related to the package name.

We have until COB Oct 24 to get this in Bioc 3.8. These last issues shouldn't take much time - we should be able to make it.

Thanks.
Valerie

@nuno-agostinho
Copy link
Author

Hey @vobencha, thank you for the quick reply. I would like to make the following comments on your points:

All man pages need running examples if possible; if there are exceptions please explain.

Do man pages for internal functions also need examples?

Regarding exported functions with no runnable examples, there are 3: downloadENCODEsamples() and downloadL1000data() have no runnable examples as they take time to download the data; loadL1000perturbations() requires the data downloaded from those functions and I thought it would be more useful to the user to have a non-runnable example with the code to download the needed files and running the function in question.

Consider renaming the vignette to something more descriptive related to the package name.

The vignette (previously named CTRAP: tutorial) will be renamed to cTRAP: differential expression comparison with cellular perturbations.

@vobencha
Copy link

vobencha commented Oct 23, 2018

Runable examples are only needed on man pages of exported functions.

WRT the download functions, I agree you don't want to download during the nightly builds. You could put a code chunk there and wrap in interactive():

if (interactive()) {
  # run the example
 }

Re: vignette name, I was specifically thinking of the R code file name. Currently the filename is tutorial.Rmd.

@nuno-agostinho
Copy link
Author

You could put a code chunk there and wrap in interactive()

I will replace the \dontrun{ with if (interactive() ) { in the examples of the functions downloadENCODEsamples(), downloadL1000data() and loadL1000perturbations().

Re: vignette name, I was specifically thinking of the R code file name. Currently the filename is tutorial.Rmd.

I will rename the vignette to comparing_DGE_with_perturbations.Rmd.

As you suggested in your previous comment, I will also remove the vignettes/tutorial.R file. Do you agree with these changes?

@bioc-issue-bot
Copy link
Collaborator

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

2611ec8 Minor changes - Update vignette title - Fix docum...

@bioc-issue-bot
Copy link
Collaborator

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.

@vobencha
Copy link

Looks good! Thanks.

@vobencha vobencha added 3a. accepted will be ingested into Bioconductor daily builder for distribution and removed 2. review in progress assign a reviewer and a more thorough review of package code and documentation taking place labels Oct 23, 2018
@bioc-issue-bot
Copy link
Collaborator

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
Copy link
Contributor

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/nuno-agostinho.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(\"cTRAP\"). The package 'landing page' will be created at

https://bioconductor.org/packages/cTRAP

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3a. accepted will be ingested into Bioconductor daily builder for distribution OK
Projects
None yet
Development

No branches or pull requests

4 participants