Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

SingleCellSignalR #1332

Closed SCA-IRCM closed 4 years ago

SCA-IRCM commented 4 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 4 years ago

Hi @SCA-IRCM

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: SingleCellSignalR
Title: Cell Signalling Using Single Cell RNAseq Data Analysis
Version: 0.99.0
Authors@R: c(person("Simon", "Cabello-Aguilar", email = "simon.cabello-aguilar@inserm.fr", role = c("aut")), person("Jacques", "Colinge", email = "jacques.colinge@inserm.fr", role = c("aut", "cre")))
Description: Allows single cell RNA seq data analysis, clustering, creates internal network and infers cell-cell interactions.
Depends: R (>= 3.6)
License: GPL-3
Encoding: UTF-8
LazyData: true
Imports:
  BiocManager,
  circlize,
  limma,
  igraph,
  gplots,
  grDevices,
  edgeR,
  SIMLR,
  data.table,
  pheatmap,
  stats,
  Rtsne,
  graphics,
  stringr,
  foreach,
  multtest,
  scran,
  utils,
RoxygenNote: 7.0.0
Suggests: 
    knitr,
    rmarkdown
VignetteBuilder: knitr
biocViews: SingleCell, Network, Clustering, RNASeq, Classification

Add SSH keys to your GitHub account. SSH keys will are used to control access to accepted Bioconductor packages. See these instructions to add SSH keys to your GitHub account.

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

mtmorgan commented 4 years ago

robust and interoperable code implies reuse of standard data representations, in particular the SingleCellExperiment class / package. Please update your code to use this.

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

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

8888740 Update Version

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

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

bf34c9b Version bump

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

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

9b656cc Update

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

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

23f4174 Update

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

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

89913fa Update

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

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

9471a3b Update

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

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

0ec01bb Update

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

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

e3dedf8 Update

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

hpages commented 4 years ago

Hi @SCA-IRCM ,

The naming style of your functions is inconsistent:

> ls('package:SingleCellSignalR')
 [1] "cell_classifier"      "cell_signaling"       "cluster_analysis"    
 [4] "clustering"           "data_prepare"         "example_dataset"     
 [7] "expression.plot"      "expression.plot.2"    "inter_network"       
[10] "intra_network"        "LRdb"                 "markers"             
[13] "markers.default"      "mm2Hs"                "mv_interactions"     
[16] "PwC_ReactomeKEGG"     "score"                "simplifyInteractions"
[19] "visualize"           

Please choose a style and stick to it.

Don't use a generic name like visualize for a very specialized plotting function.

Your score function will conflict with the score generic function defined in the BiocGenerics package. (Bioconductor is an ecosystem and you want to make sure that your package fits nicely in it by cohabiting peacefully with the other packages in the ecosystem.)

Plotting function expression.plot.2 doesn't plot anything.

Thanks!

bioc-issue-bot commented 4 years ago

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

b9810b0 Update

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

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

a36f624 Update

bioc-issue-bot commented 4 years ago

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

0b20ebc Update

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

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

4b3f82f Update

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

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

fe1c457 Update

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

hpages commented 4 years ago

Hello?

SCA-IRCM commented 4 years ago

Hello?

SCA-IRCM commented 4 years ago

Yes what can I do for you?

hpages commented 4 years ago

Good to hear your voice ;-)

So is the package ready for review?

SCA-IRCM commented 4 years ago

Hi, I thought it was already under review. Following the indications from the message you sent on january 9th:

  1. I changed the names of the functions so their style is consistent.
  2. I changed the visualize() function into visualize_interaction()
  3. I changed the score() function into LRscore()
  4. The example in the (now called) expression_plot_2() function plots something.

Is there anything else I need to do? Otherwise I think it is ready for review :).

hpages commented 4 years ago

My message from January 9th were just some quick preliminary remarks. I'll take a closer look at the package in the next few days. Thanks!

hpages commented 4 years ago

Please address the following issues:

  1. The Contents section at the beginning of the vignette is out-of-sync with the real content. For example the name of section "Details of each function" in Contents is different from its real name. Also its nesting level in Contents is different from its real nesting level (it's listed as a subsection in Contents but is a top-level section in the document). Finally sections are numbered in Contents but not in the body of the vignette. Please use a standard mechanism to automatically generate the Contents section from the real content. The main reason this kind of mechanism exists is to avoid the out-of-sync problem (and also to save you some work).

  2. Half of the vignette is a copy/paste of the man pages of the functions defined in the package. Two problems with this: (1) this has no place in the vignette, (2) this kind of copy/paste is guaranteed to lead to an out-of-sync problem, again (but more serious in this case than with your Contents section). Note that all the man pages will be automatically compiled in a PDF document called "Reference Manual" that we make available on the package landing page. See for example: https://bioconductor.org/packages/SummarizedExperiment They do this on CRAN too! Unlike a document obtained by copy/past'ing, the Reference Manual is guaranteed to stay in sync with the real man pages.

  3. Please provide a running example for data_prepare(). FWIW you're misusing \donttest here i.e. code inside \donttest must be valid. See "2.1.1 Documenting functions" in the Writing R Extensions manual. Note that I'm NOT suggesting that you keep \donttest. What I'm asking is that you provide a running example that is NOT inside a \donttest or \dontrun so R CMD check and example(data_prepare) will both run it.

Thanks!

SCA-IRCM commented 4 years ago

Hello, Thank's for your review, I'll correct everything today. I just have a question/remark for the point 2. It is not just a copy/paste of the man pages of the function. Much more details are provided in the section called Details, do I copy/paste this information into the @return section of every function so that will appear in the Reference Manual? I would really appreciate a quick answer so I can finish this as soon as possible. Thank you in advance.

bioc-issue-bot commented 4 years ago

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

cb445db Updates

SCA-IRCM commented 4 years ago

Hi, I fixed the three points you raised. For the point 2. I finally added a @details sections in the functions descriptions, I think/hope it will be OK :). I wait for your confirmation. Yours,

SCA

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

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

2ad4329 Update DESCRIPTION

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

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

1c1b263 Update

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

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

dee543b Updates

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

hpages commented 4 years ago

The man page of a function is where you put everything you want to say about it. It doesn't really make sense to have 2 versions of a man page: a short/incomplete one accessible via ?cell_classifier and an extended/complete one in the vignette. The user really expects to see all the information in the standard place, which is the real man page (i.e. the page accessible via ?cell_classifier). So yes please merge whatever you have in the vignette into the real man pages and remove the "extended man pages" from the vignette. This duplication is not needed and is almost certain to cause problems in the long run.

More generally speaking, you should apply the DRY principle, which is an important aspect of good software engineering. It also applies to the documentation of the software.

Thanks!

SCA-IRCM commented 4 years ago

Yes of course, I understand what you mean. I proceeded to the consequent changes and the package is ready to review. Thank you!