Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

ClustIRR #3087

Closed snaketron closed 1 year ago

snaketron commented 1 year 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 questions/help about the submission process, including questions about the output of the automatic reports generated by the SPB (Single Package Builder), please use the #package-submission channel of our Community Slack. Follow the link on the home page of the Bioconductor website to sign up.

bioc-issue-bot commented 1 year ago

Hi @snaketron

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: ClustIRR
Type: Package
Title: Clustering of immune receptor repertoires
Version: 0.99.0
Authors@R: c(
    person("Simo", "Kitanovski", email = "simo.kitanovski@uni-due.de", 
    role = c("aut", "cre"), comment=c(ORCID="0000-0003-2909-5376")),
    person("Kai", "Wollek", email = "kai.wollek@uni-duisburg-essen.de", 
    role = c("aut"), comment=c(ORCID="0009-0008-5941-9160")))
Description: ClustIRR is a quantitative method for clustering of immune  
    receptor repertoires (IRRs). 
    The algorithm of ClustIRR finds groups of T or B cell receptors (TCRs or 
    BCRs) that likely have similar specificity. 
    This is achieved by comparing the global and local sequence features of 
    complementarity determining regions (CDRs). 
    Once the specificity groups are identified, ClustIRR visualizes them as 
    using graphs.
License: GPL-3 + file LICENSE
LazyData: false
Depends:
    R (>= 4.2.0)
Imports:
    stringdist,
    future,
    future.apply,
    methods,
    stats,
    utils,
    igraph,
    visNetwork
Suggests:
    BiocStyle,
    knitr,
    testthat,
    ggplot2,
    patchwork,
    ggrepel
Encoding: UTF-8
NeedsCompilation: no
biocViews: Clustering, ImmunoOncology, SingleCell, Software, Classification
RoxygenNote: 7.2.3
VignetteBuilder: knitr
URL: https://github.com/snaketron/ClustIRR
BugReports: https://github.com/snaketron/ClustIRR/issues
bioc-issue-bot commented 1 year ago

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

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. It is required to push a version bump to git.bioconductor.org to trigger a new build.

Bioconductor utilized your github ssh-keys for git.bioconductor.org access. To manage keys and future access you may want to active your Bioconductor Git Credentials Account

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

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.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.0.tar.gz macOS 12.6.5 Monterey: ClustIRR_0.99.0.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 692bb2749d48e690f66960c0d996f651e975658e

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

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.

The following are build products from R CMD build on the Bioconductor Build System: macOS 12.6.5 Monterey: ClustIRR_0.99.1.tar.gz Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.1.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: e52b2d5c0fb3829e214cca21835419d5e43decfd

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: macOS 12.6.5 Monterey: ClustIRR_0.99.3.tar.gz Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.3.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 0b22f5cab2ec10be44f2b0a4f0fff65c762b39a0

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.4.tar.gz macOS 12.6.5 Monterey: ClustIRR_0.99.4.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

jianhong commented 1 year ago

Package 'ClustIRR' Review

Thank you for submitting your package to Bioconductor. The package passed check and build. However there are several things need to be fixed. Please try to answer the comments line by line when you are ready for a second review. Code: Note: please consider; Important: must be addressed.

The DESCRIPTION file

Others

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 68bd6e7d1d34f02d9a983081100db08cbcf83dbe

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

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.

The following are build products from R CMD build on the Bioconductor Build System: macOS 12.6.5 Monterey: ClustIRR_0.99.5.tar.gz Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.5.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 92d7c9cacba103d10f3c5949adb4042ab17141e3

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.7.tar.gz macOS 12.6.5 Monterey: ClustIRR_0.99.7.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

snaketron commented 1 year ago

The NAMESPACE file

  • [x] Selective imports using importFrom instead of import all with import.

    • in line 3 import(future)
    • in line 4 import(future.apply)
    • in line 7 import(stringdist)
    • in line 9 import(visNetwork)

Fixed in previous (commit id: 92d7c9cacba103d10f3c5949adb4042ab17141e3) push.

snaketron commented 1 year ago
  • [x] NOTE: :: is not suggested in source code unless you can make sure all the packages are imported. Some people think it is better to keep ::. However please note that you need to manully double check the import items when you make any change in the DESCRIPTION file during development. My recommendation is to remove one or two repeats to force the dependency check.

We have removed :: and double-checked the dependencies. Only in the case of the package future we left the :: symbols in local_clust.R. Without this we encountered errors documented earliers by the developers of future here: https://github.com/HenrikBengtsson/future/issues/152

The DESCRIPTION file

  • [x] Important: R version should be no less than 4.3

Both fixes are provided in the following commit id: 55fbfb4a08d78befa6e579f80795287ed5ecb98e

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 55fbfb4a08d78befa6e579f80795287ed5ecb98e

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.8.tar.gz macOS 12.6.5 Monterey: ClustIRR_0.99.8.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: e28534d62aca60dc340bab7c2d3ccff8503c8070

snaketron commented 1 year ago

Documentation

  • [x] Important: Vignette should have an Installation section.

    • rmd file vignettes/User_manual.Rmd
  • [x] Important: Please include Bioconductor installation instructions using BiocManager.

    • rmd file vignettes/User_manual.Rmd
  • [x] Note: Vignette includes motivation for submitting to Bioconductor as part of the abstract/intro of the main vignette.

    • rmd file vignettes/User_manual.Rmd

We included a Installation section in the vignette.

Commit id: e28534d62aca60dc340bab7c2d3ccff8503c8070

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: macOS 12.6.5 Monterey: ClustIRR_0.99.9.tar.gz Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.9.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 445c4a0331e5643d5db837a60c8ee67419a7a09c

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.11.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

snaketron commented 1 year ago
  • [x] Note: Use S4 object instead of S3 object.
  • In file R/cluster.R:

    • at line 60 found ' return(base::structure('

Using object oriented programming was not necessary at all. We needed a simple list with two elements as output and this is what we did.

snaketron commented 1 year ago
  • [x] NOTE: Functional programming: code repetition.

We reduced the code repetition.

While it is possible to reduce the code repetition in the functions in input_check.R, we think that the code repetition here is an asset: 1) it improves readability while we check the individual parameters 2) while ClustIRR is mature in terms of its functionality, we can see many directions in which ClustIRR may soon evolve (acquire new input parameters as well). Adding new input parameters or removing the old ones is easily done given that the different parameters are treated/checked independently of each other. 3) the main drawback, as mentioned by the reviewer, is code redundancy

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 1d6132fd69002b63ad53c2112f5197ed4cccc6a2

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

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.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.12.tar.gz macOS 12.6.5 Monterey: ClustIRR_0.99.12.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 4d2d95d97c8514f4685f1f1bea9e8eeace3b5c41

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: macOS 12.6.5 Monterey: ClustIRR_0.99.14.tar.gz Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.14.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

snaketron commented 1 year ago

General package development

  • [ ] Important: Package files exceed the 5MB size limit.

Dear @jianhong,

thank you for the review of our package.

We have addressed all important comments except for the one above (we do not find this error as all files in the package are smaller than 5mb). The file you are referencing appears to be an .git file. Right? As far as I understand git removing this file could be damaging.

Furthermore, we addressed nearly all notes. Our comments on the individual issues are shown above.

jianhong commented 1 year ago
  • [x] Note: Use S4 object instead of S3 object.
  • In file R/cluster.R:

    • at line 60 found ' return(base::structure('

Using object oriented programming was not necessary at all. We needed a simple list with two elements as output and this is what we did.

If you do not want to change to S4 object, please keep the S3 object. You changed in your code part but did not change the documentation. I still suggest you to change it into S4 object because your plot_graph is depend on this object. If you use S4 object, you can simply merge the check_clustirr into the object construction step. But this is not required.

jianhong commented 1 year ago

Other minor issues:

R code

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 098d67fb8a7e1529f60d3317ed1e84db9057cb83

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.16.tar.gz macOS 12.6.5 Monterey: ClustIRR_0.99.16.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

snaketron commented 1 year ago

Dear @jianhong

in the latest push we addressed both issues (R code and S4 object)

jianhong commented 1 year ago

Namespace

R code

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 55689dae4532a416575732b3a0e9c4dec5135afe

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

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.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.17.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 531e13aa1969e4b94f6e432d0f3457809df00c98

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.18.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

snaketron commented 1 year ago

Dear @jianhong

the S4 object related issues should be solved now.

jianhong commented 1 year ago

You may misunderstand the slot access comments. Please do not use @ or slot outside of accessors. See reference here and here for complete implementation of a S4 object.

snaketron commented 1 year ago

@jianhong I implemented the code according to Hadley Wickham's tutorial 'The S4 object system' (you refer to this tutorial)

Text from the tutorial:

To access slots of an S4 object you use @, not $: hadley <- new("Person", name = "Hadley") hadley@age Or if you have a character string giving a slot name, you use the slot function: slot(hadley, "age")

In our case the object is a multifactorial result from our packages' main function. We expect the user to access directly the two slots of this object. And yes, we can implement two accessors that do the same thing slots does, but this looks like an overkill in our particular example.

I completely agree with the idea stated in the Biocconductor guidelines:

Generally @ should only be used in an accessor, all other code should use the accessor. The accessor does not need to be exported from the class if the user has no need or business accessing parts of your data object.

However, here we are not hiding anything from the user, i.e. the two slots that are available are exported and should be directly accessible.

Please let me know what you think.

jianhong commented 1 year ago

Does that mean you never though any possibility the others will import your package and re-use the S4 class?

snaketron commented 1 year ago

I am quite certain that we will re-use the S4 class in the future, most likely in some new functions of this package.

Not sure about others. My guess is that others will primarily apply ClustIRR to analyze their data. Currently, we see few R-packages that deal with downstream analysis TCR-seq data, so it is difficult to imagine how other developers will react to this package.

jianhong commented 1 year ago

If answer is yes, then the accessor are encouraged.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: f664363cd0b60050ef06a4f9900e59137215fe27

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

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.

The following are build products from R CMD build on the Bioconductor Build System: macOS 12.6.5 Monterey: ClustIRR_0.99.21.tar.gz Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.21.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 1d7727ecd960fb987f96be261b64c20a30c3b3ed

bioc-issue-bot commented 1 year ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): ClustIRR_0.99.22.tar.gz macOS 12.6.5 Monterey: ClustIRR_0.99.22.tar.gz

Links above active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/ClustIRR to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.