search

Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

submitting new package CatsCradle #3452

Closed michaeldshapiro closed 3 hours ago

michaeldshapiro commented 3 months 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 3 months ago

Hi @michaeldshapiro

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: CatsCradle
Title: This package discovers gene clusters in Seurat data
Version: 0.99.0
Authors@R:
    person(given = "Anna",
        family = "Laddach",
        role = c("aut", "cre"),
        email = "anna.laddach@crick.ac.uk",
        comment = c(ORCID = "0000-0001-5552-6534"))
    person(given = "Michael",
        family = "Shapiro",
        role = c("aut", "cre"),
        email = "michael.shapiro@crick.ac.uk",
        comment = c(ORCID = "0000-0002-2769-9320"))
Description: The basic insight driving this package is the idea that by transposing the expression matrix of a Seurat object, we can use the usual Seurat technology to cluster genes and to visualise their relationships in two dimensions, e.g., as UMAP plots.  In addition we are then able to use the nearest neighbor graph to gain additional insight into specific genes by querying their neighborhood in this graph. 
License: MIT + file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.1
Imports: Seurat,
     ggplot2,
     networkD3,
     stringr,
     pracma,
     knitr,
     reshape2,
     rdist,
     igraph,
     interp,
     geometry,
     Rfast,
     dplyr,
     data.table,
     abind,
     pheatmap,
     fossil
Depends: 
    R (>= 4.0)
LazyData: true
VignetteBuilder: knitr
LazyDataCompression: xz
biocViews:
    BiologicalQuestion,
    StatisticalMethod,
    GeneExpression,
    SingleCell,
    Transcriptomics
lshep commented 3 months ago

The docs directory should be removed. These documentations should be generated automatically with R CMD build.

Seurat objects are not Bioconductor objects. This package should be altered to additionally be able use the main Bioconductor object for expression matrix, SummarizedExperiment/SingleCellExperiment.

Please also be sure to run R CMD build, R CMD check and BiocCheck on the package. 

> cellTypesPerCellType = computeCellTypesPerCellTypeMatrix(NBHDByCTMatrix,
+                                                      smallXenium$seurat_clusters)
Error in `x[[i, drop = TRUE]]`:
! ‘seurat_clusters’ not found in this Seurat object

Backtrace:
     ▆
  1. ├─CatsCradle::computeCellTypesPerCellTypeMatrix(...)
  2. │ ├─stats::aggregate(nbhdByCellType, list(cellTypes), sum)
  3. │ └─stats::aggregate.data.frame(...)
  4. ├─smallXenium$seurat_clusters
  5. └─SeuratObject:::`$.Seurat`(smallXenium, seurat_clusters)
  6.   ├─x[[i, drop = TRUE]]
  7.   └─SeuratObject:::`[[.Seurat`(x, i, drop = TRUE)
  8.     └─base::tryCatch(...)
  9.       └─base (local) tryCatchList(expr, classes, parentenv, handlers)
 10.         └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 11.           └─value[[3L]](cond)
 12.             └─rlang::abort(...)
michaeldshapiro commented 3 months ago

We have pushed a new version of the package with the updated version number 0.99.1. We believe this version responds successfully to the suggestions. In particular, it now passes R CMD check and BiocCheck, and we have removed to doc folder.

We like the idea of enabling the package to work with, e.g., SingleCellExperiment objects. Functions which accept Seurat objects now also accept equivalent data in the form of a SingleCellExperiment of SpatialExperiment object. Similarly, functions which return as Seurat object can now return a SingleCellExperiment of SpatialExperiment at the user's request. This has been accomplished using adaptors that are only slightly deeper than, e.g., as.SingleCellExperiment. We have also provided expamples showing this capability.

We think this raises a larger question for the community. Seurat is an industry standard and will continue to evolve. We wonder if there oughtn't be a package which provides deeper conversions between Seurat and Bioconductor objects. This is beyond the scoper of CatsCradle and deserves to be treated as its own project.

Many thanks for your review of our work.

lshep commented 3 months ago

Agreed regarding Seurat/Bioc object conversion. https://satijalab.org/seurat/archive/v4.3/conversion_vignette looks like there was at least some effort to make some standardized conversions between objects; however I don't know how mature it is or if a deeper effort is being investigated.

bioc-issue-bot commented 3 months ago

Your package has been added to git.bioconductor.org to continue the pre-review process. A build report will be posted shortly. Please fix any ERROR and WARNING in the build report before a reviewer is assigned or provide a justification on why you feel the ERROR or WARNING should be granted an exception.

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. All changes should be pushed to git.bioconductor.org moving forward. It is required to push a version bump to git.bioconductor.org to trigger a new build report.

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 3 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "WARNINGS, 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 Single Package Builder: Linux (Ubuntu 22.04.3 LTS): CatsCradle_0.99.1.tar.gz macOS 12.7.1 Monterey: CatsCradle_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/CatsCradle to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

michaeldshapiro commented 2 months ago

We believe we have resolved the build issues and have pushed a new version and bumped the version number.

One of the issues involved the size of the tarball. We found that bringing the tarball down under the 5M limit required severe sugery on one of our data objects. Single cell data can be large. In particular, getting the tarball down to size required removing enough features that this restricted some of the documentation we can provide. We wonder if it isn't time to increase the size limit on tarballs and wonder where is the best place to raise this issue.

michaeldshapiro commented 2 months ago

We pushed a new version with a new version number almost a week ago, but this does not seem to have triggered a new build. Accordingly, we have pushed a new version with increased version number. Is there something different we should be doing to trigger a build?

PeteHaitch commented 2 months ago

It looks like you haven't pushed those recent updates to git@git.bioconductor.org:packages/CatsCradle but only to your GitHub-hosted repo?

% git clone git@git.bioconductor.org:packages/CatsCradle
Cloning into 'CatsCradle'...
Enter passphrase for key '/Users/peter/.ssh/id_ed25519': 
remote: Enumerating objects: 2010, done.
remote: Counting objects: 100% (2010/2010), done.
remote: Compressing objects: 100% (1006/1006), done.
remote: Total 2010 (delta 976), reused 2010 (delta 976), pack-reused 0
Receiving objects: 100% (2010/2010), 406.38 MiB | 5.89 MiB/s, done.
Resolving deltas: 100% (976/976), done.
% git log -n 1
commit 627d28528b6c5bc563ab35b9066fc3d52ecb7f20 (HEAD -> devel, origin/devel, origin/HEAD)
Author: AnnaLaddach <annaladdach@gmail.com>
Date:   Wed Jun 26 11:14:14 2024 +0100

    S and S transpose fix

Please read this documentation for setting up remotes to push to git.bioconductor.org. All changes should be pushed to git.bioconductor.org moving forward. It is required to push a version bump to git.bioconductor.org to trigger a new build report.

bioc-issue-bot commented 2 months ago

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

bioc-issue-bot commented 2 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "WARNINGS, 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 Single Package Builder: macOS 12.7.1 Monterey: CatsCradle_0.99.4.tar.gz Linux (Ubuntu 22.04.3 LTS): CatsCradle_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/CatsCradle to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 months ago

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

bioc-issue-bot commented 2 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "WARNINGS, 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 Single Package Builder: macOS 12.7.1 Monterey: CatsCradle_0.99.5.tar.gz Linux (Ubuntu 22.04.3 LTS): CatsCradle_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/CatsCradle to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 months ago

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

bioc-issue-bot commented 2 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

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.

The following are build products from R CMD build on the Single Package Builder: ERROR before build products produced.

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/CatsCradle to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 months ago

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

bioc-issue-bot commented 2 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "WARNINGS, 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 Single Package Builder: macOS 12.7.1 Monterey: CatsCradle_0.99.7.tar.gz Linux (Ubuntu 22.04.3 LTS): CatsCradle_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/CatsCradle to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 2 months ago

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

bioc-issue-bot commented 2 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

On one or more platforms, the build results were: "WARNINGS, 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 Single Package Builder: macOS 12.7.1 Monterey: CatsCradle_0.99.8.tar.gz Linux (Ubuntu 22.04.3 LTS): CatsCradle_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/CatsCradle to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

michaeldshapiro commented 2 months ago

We need some help understanding an error message in the build report. This occurs in the BiocCheck section on coding practices:

Error in seq.default(which(cond) - length(notLookback), which(cond)) : 'from' must be of length 1

We have replaced all occurences of the form 1:N with seq_len(N) and all occurences of the form 2:N with seq(from=2,to=N), etc, Furthermore, we do not see this error when running BiocCheck() locally.

Any help would be most appreciated.

michaeldshapiro commented 2 months ago

Our understanding is that the build error we are getting is due to a bug in BiocCheck. Unless someone can point to a specific problem with our code, we think it would be appropriate to move this project on to the next stage.

bioc-issue-bot commented 2 months ago

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

bioc-issue-bot commented 2 months ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

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 Single Package Builder: macOS 12.7.1 Monterey: CatsCradle_0.99.9.tar.gz Linux (Ubuntu 22.04.3 LTS): CatsCradle_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/CatsCradle to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

michaeldshapiro commented 2 months ago

As regards the WARNING regarding the CITATION file. We would be happy to revise this when CatsCradle is available on Bioconductor. However, the package is already being used by multiple researchers who are accessing it from github and we would like to give them something to cite.

bioc-issue-bot commented 1 month ago

A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.

HelenaLC commented 1 month ago

main

misc

docs

code

bioc-issue-bot commented 1 month ago

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

michaeldshapiro commented 1 month ago

Dear Helena

Thank you for giving our code a well-considered review. We have no question that this has helped to improve our package. In particular, we have made an effort to make this package more compatible with the SingleCellExperiment and SpatialExperiment classes. Before we get into specifics about this we'd like to address the elephant in the room.

We certainly understand the value of the inter-operability of Bioconductor classes and code. But at this point, Seurat is pretty much the standard for working with single cell data in R. The functionality that we have provided for working between Seurat on the one hand and SingleCellExperiment and SpatialExperiment on the other provides only a minimal extension to the off-the-shelf functions as.Seurat and as.SingleCellExperiment. What is needed here is a Bioconductor package devoted to inter-operability between these packages. This is a sizable undertaking, one well outside the scope of CatsCradle, and one which needs a dedicated team. But it would contribute significantly to a more sympatico relationship between these two worlds.

On to specifics:

All vignettes use a SeuratObject to demo capabilities. As pointed out previously, Seurat is not on Bioc. I appreciate the sentiment to assure usability across communities, however, please also include a vignette designated to using Bioc infrastructure, i.e., the SingleCellExperiment class alongside corresponding visualizations, e.g., using scater.

I noticed that predictAnnotation (possibly other functions) runs instantly for the SeuratObject , but stalls for the (identical) SingleCellExperiment . I traced this down to acceptor() , which calls on SCEtoSeurat , which is extremely inefficient. Granted this is exactly what is assuring interoperability with Bioc, it'd be much appreciated to assure that functions work equally on both types of objects (in terms of results and computation speed/cost).

In the DESCRIPTION, consider adding "Spatial" to biocViews: .

In the README, please add installation instructions using BiocManager::install() . We will be more than happy to do this when the package is approaching availability on Bioconductor. However, at the moment, we already have users in other labs accessing CatsCradle from github. Consider adding a NEWS file to keep track of changes, in non-technical language, in the code from one release version to the next (see here).

Seurat has been highly volatile over the years. When depending on it, I'd suggestion specifying exactly which version is expected to be compatible with your package (i.e., Seurat (>= x.y.z) ).

Please consider implementing unit tests; we strongly encourage them!

Please address build report NOTES to the best of you abilities; e.g., LazyData: in the DESCRIPTION should be set to false or removed, R version dependency should be >= 4.4.0, add URL: and BugReports: fields, add the output of the sessionInfo() at the vignette's conclusion, and more.

"S" is a rather unfortunate name for a package's data object (not unlikely that data(S) would overwrite the user's variable)... consider changing this to something more explicit, e.g., exSeuratObj or something.

Data documentation is a little brief in some instances, e.g., humanLRN is missing source information or similar. Please see here for guidance, and include provenance and source information of all objects (accompanied by an inst/script/ , if applicable).

I'd suggest consistently using code style for anything R-related (e.g., variable, function, class names) etc. -- if applicable, you can hyperlink external packages using BiocStyle functions, e.g., r BiocStyle::CRANpkg("ggplot2") and r BiocStyle::Biocpkg("scuttle") for packages hosted via CRAN and Bioc, respectively.

All functions are currently placed in 2 scripts with 1500 lines+ This makes things very hard to grasp, review, maintain. Consider placing functionally related code/helpers in separate scripts and, if applicable, using designated, say, utils-plotting.R, utils-spatial.R etc. scripts for code used repeatedly.

In the build report, there are numerous WARNING related to namespace clashes (packages exporting identically named things). Most of these should be resolved by selective importing, which is in any case recommended; i.e., using @importFrom package function rather than @import to include a packages full namespace (see here).

Please also address the NOTE related to unused imports, namely, fossil,interp,knitr (the latter should go under Suggests: ).

NOTE on "partial argument match of 'meta' to 'meta.data'" CreateSeuratObject() should be resolved with CreateSeuratObject(..., meta.data=...) .

Note on "" should be addressed to the best of your abilities. E.g., median is not being imported (need to add @importFrom stats median ) etc.

Some examples have extremely long runtimes (~30s vs. ~5s recommended); please try and reduced these.

-----Original Message----- From: Bioconductor/Contributions @.> Sent: Aug 8, 2024 11:13 AM To: Bioconductor/Contributions @.> Cc: michaeldshapiro @.>, Mention @.> Subject: Re: [Bioconductor/Contributions] submitting new package CatsCradle (Issue #3452)

main * All vignettes use a SeuratObject to demo capabilities. As pointed out previously, Seurat is not on Bioc. I appreciate the sentiment to assure usability across communities, however, please also include a vignette designated to using Bioc infrastructure, i.e., the SingleCellExperiment class alongside corresponding visualizations, e.g., using scater.

misc * In the DESCRIPTION, consider adding "Spatial" to biocViews:.

docs * Data documentation is a little brief in some instances, e.g., humanLRN is missing source information or similar. Please see here (https://contributions.bioconductor.org/docs.html#doc-inst-script) for guidance, and include provenance and source information of all objects (accompanied by an inst/script/, if applicable).

code * In the build report, there are numerous WARNING related to namespace clashes (packages exporting identically named things). Most of these should be resolved by selective importing, which is in any case recommended; i.e., using @importFrom package function rather than @import to include a packages full namespace (see here (https://contributions.bioconductor.org/namespace.html?q=importfrom#imported-functions)).

— Reply to this email directly, view it on GitHub (https://github.com/Bioconductor/Contributions/issues/3452#issuecomment-2275455760), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AL4ZXLBFEOB5YZDHAOAHUZLZQNADZAVCNFSM6AAAAABIYAYED6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZVGQ2TKNZWGA). You are receiving this because you were mentioned.Message ID: @.***>

bioc-issue-bot commented 1 month ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

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 Single Package Builder: Linux (Ubuntu 22.04.3 LTS): CatsCradle_0.99.10.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/CatsCradle to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

HelenaLC commented 1 month ago

Dear Michael, apologies in advance for the annoyance this might cause, but I hope you can understand our sentiment here from a 20+ years established community perspective...

In it's current shape, the package focuses on Seurat structures (as pointed out during pre-review already). -- reiterating core-team members here: -- With Seurat on CRAN, it does seem natural for your package to be on CRAN, too. Our ecosystem is striving for convenience to users and developers through direct reuse of established and essentially stable data structures like SingleCellExperiment. We are hitting resource limits for reviewing and QCing packages and so we would ask that if you want to have the package in Bioconductor, you enhance the direct interfaces to Bioc data structures directly (under-the-hood conversion and method dispatch to Seurat only is not sufficient); thanks!

Fwiw, SPOTlight is a good example of writing methods that work on basic data structures (like vectors/matrices), which are pulled out from SeuratObject/SingleCellExperiment objects, allowing for direct method dispatch to both.

michaeldshapiro commented 1 month ago

Thank you for clarifying that Bioconductor is not the right place for our package. We certainly feel that the “traditional” approach and need to rigidly stick to established data structures will not be tenable given the fast-paced development of single-cell omics technology. Furthermore, we are saddened that Bioconductor would wish to remain an island rather than to interface with the plethora of high-quality tools available on other repositories. As our user base currently uses Seurat for single-cell analysis we feel it is essential for us to maintain compatibility with this package, and would like to point out that use of Seurat is more widespread than SingleCellExperiment, as evidenced by its citation record.

We put considerable effort into addressing the initial review comments and ensured a seamless interface with existing Bioconductor structures. Furthermore, in the initial review, we were not asked to remove Seurat from the dependencies but rather to specify the exact version required. To issue a review that fails to acknowledge that we made the initial requested changes suggests the above-mentioned resource limits to effectively review have already been passed. Beyond this, we feel Bioconductor must clarify its stance on external packages. To the best of our knowledge, most Bioconductor packages import multiple CRAN packages. If it is specifically CRAN single-cell packages that should not be imported, this should be made clear in the Bioconductor documentation.

We hope that our feedback will lead to changes in the Bioconductor documentation that will improve the convenience of the Bioconductor development process, perhaps by including guidelines/a list of “prohibited” packages that should not be interfaced with. We initially wished to submit our package to Bioconductor due to its policy of valuing an “open approach to science that promotes sharing of ideas, code, software and expertise”. Unfortunately, we now realise we were mistaken.

Anna Laddach and Michael Shapiro

vjcitn commented 1 month ago

Let's continue the discussion, OK? Reviewing these packages is hard and is done on top of huge workloads for Bioc core members and volunteer reviewers. I would like to take a look at the series of comments, at the code, and have at least one more discussion.

vjcitn commented 1 month ago

Really sorry that this is going in this direction. These remarks 

We certainly feel that the “traditional” approach and need to rigidly stick to established data structures 
will not be tenable given the fast-paced development of single-cell omics technology. Furthermore, we are 
saddened that Bioconductor would wish to remain an island rather than to interface with the 
plethora of high-quality tools available on other repositories.

mention "tradition", "rigidity", and "island" in ways that I personally find unwelcome in the context of project effort. I accept that it is difficult to receive reviewer inputs that imply even more work must be done on a package that you feel is complete. But the comments given above reflect significant misconceptions about the purpose and position of the Bioconductor project.

I examined the vignette and find that the statement

Every function which accepts a Seurat object as input can also accept
a SingleCellExperiment in its place.

found at the end of the quick start document is not correct. Specifically

> sst = as.SingleCellExperiment(STranspose)
> cat = getNearbyGenes(STranspose,'Myc',radius=1,metric='NN',weights=TRUE)
> cat2 = getNearbyGenes(sst,'Myc',radius=1,metric='NN',weights=TRUE)
Error in as(f@graphs[[graph]], "TsparseMatrix") :
  no method or default for coercing “NULL” to “TsparseMatrix”

We thank you for your submission. Section 4.2.2 of guidelines is explicit about the importance of explicit interoperation with Bioconductor classes. If you are continuing with submission to CRAN, please close this issue.

vjcitn commented 1 month ago

I just noticed the special vignette on interop with SingleCellExperiment. That's nice but coercion still imposes burdens on users that can be absorbed by interoperable development.

michaeldshapiro commented 1 month ago

Let's pursue this further on Monday. The magic word here is "interoperable development".

-----Original Message----- From: Bioconductor/Contributions @.> Sent: Aug 30, 2024 6:22 PM To: Bioconductor/Contributions @.> Cc: michaeldshapiro @.>, Mention @.> Subject: Re: [Bioconductor/Contributions] submitting new package CatsCradle (Issue #3452)

I just noticed the special vignette on interop with SingleCellExperiment. That's nice but coercion still imposes burdens on users that can be absorbed by interoperable development. — Reply to this email directly, view it on GitHub (https://github.com/Bioconductor/Contributions/issues/3452#issuecomment-2322020645), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AL4ZXLHPAT7WE6PG7A3VH33ZUCS5TAVCNFSM6AAAAABIYAYED6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRSGAZDANRUGU). You are receiving this because you were mentioned.Message ID: @.***>

michaeldshapiro commented 4 weeks ago

First let me address your comment on the failure to of getNearbyGenes() to run on your object sst. This is an as.SingleCellExperiment() shortcoming, not a CatsCradle shortcoming. as.SingleCellExperiment() is a very shallow adaptor which does not copy over the nearest neighbour graphs. This is why we have written the only slightly less shallow function SeuratToSCE(). We didn't export it because it's still very shallow, but maybe we should? In any case, if you will run your code on STranspose_sce = getExample('STranspose_sce') it will work fine.

michaeldshapiro commented 4 weeks ago

It seems to me that there are two issues here. One is the question of Bioconductor's broader relationship to Seurat, whether there is a place on Bioconductor for packages which expect much of their traffic to be in Seurat objects and how those packages should handle those objects. These are absolutely worthwhile questions and we're more than happy to discuss them in a separate issue. But we feel they are no longer in play in this review.

Why is that? Because we were given a detailed list of requests and we put a significant amount of work into responding to those requests. At that point, the question becomes, "Did we do an adequate job of responding to those requests?" Our comment of Aug 28 gives an exhaustive verbatim listing of those requests together with our response to each.

If you wish to continue with review of this package, we would suggest that the appropriate course of action would be to proceed from that list and treat everything since as a bad dream from which we have finally awaken.

We await your reply.

michaeldshapiro commented 3 weeks ago

The radio silence seems to belie the offer to "continue the discussion". It is now abundantly clear that Bioconductor is either unwilling or unable to provide an intellectually defensible review of this project. Our comments of Aug 30 are only confirmed by events since.

vjcitn commented 3 weeks ago

hi -- it has been a holiday and there are lots of things going on ... sorry not to keep a close eye on this. let me read more closely.

vjcitn commented 3 weeks ago

if the issue is closed, the review is done. reopen it if you are still interested. we have a conference going on and there are many packages being reviewed at this time. i sense a degree of bellicosity that is unfamiliar in my work on the project, it is not helpful.

michaeldshapiro commented 3 weeks ago

We're very, very frustrated here, feel our work has been unfairly dismissed. But we're more than willing to go forward if that is not your intent. It seems I have over-interpreted your silence and for that I apologize. I will re-open.

-----Original Message----- From: Bioconductor/Contributions @.> Sent: Sep 4, 2024 1:17 PM To: Bioconductor/Contributions @.> Cc: michaeldshapiro @.>, State change @.> Subject: Re: [Bioconductor/Contributions] submitting new package CatsCradle (Issue #3452)

if the issue is closed, the review is done. reopen it if you are still interested. we have a conference going on and there are many packages being reviewed at this time. i sense a degree of bellicosity that is unfamiliar in my work on the project, it is not helpful. — Reply to this email directly, view it on GitHub (https://github.com/Bioconductor/Contributions/issues/3452#issuecomment-2328824141), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AL4ZXLEWUHVIVTTJDCRLUULZU326ZAVCNFSM6AAAAABIYAYED6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRYHAZDIMJUGE). You are receiving this because you modified the open/close state.Message ID: @.***>

bioc-issue-bot commented 3 weeks ago

Dear @michaeldshapiro ,

We have reopened the issue to continue the review process. Please remember to push a version bump to git.bioconductor.org to trigger a new build.

bioc-issue-bot commented 3 weeks ago

A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.

vjcitn commented 3 weeks ago

I am glad you reopened. We are not dismissing your work, not at all. I did make a superficial run-through and reported a little error and probably should not have, but we are firing on all cylinders in this project. We have been hit by robots downloading data for which we pay egress charges, there have been problems with certification processes for servers, and we are renovating infrastructure that nobody really sees. We are glad to interoperate with Seurat and want to ensure that all opportunities for increasing user satisfaction with Bioconductor packages (such as yours may become) are taken. I am no expert on spatial data or on Seurat but I need to know more and will take this opportunity to work more closely with your package, and I will take over the review process. @HelenaLC has done a great job with the review and your technical responses are appreciated. I do regret some of the rhetoric and bellicosity and hope that this can be put behind us. I will write more within a week. If there are particular reasons for seeking a rapid decision please make them known, you may email me directly if this is a concern.

bioc-issue-bot commented 3 weeks ago

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

michaeldshapiro commented 3 weeks ago

Many thanks for your kind and wise letter. It goes a long way towards helping me put my paranoia behind me. At least for a day or two! Thank you.

In the meantime, I have cleaned up some minor issues and am continuing to work on the running times of the examples. We'll see how that goes. I can explain some of the obstacles in due course.

-----Original Message----- From: Bioconductor/Contributions @.> Sent: Sep 4, 2024 1:55 PM To: Bioconductor/Contributions @.> Cc: michaeldshapiro @.>, Mention @.> Subject: Re: [Bioconductor/Contributions] submitting new package CatsCradle (Issue #3452)

I am glad you reopened. We are not dismissing your work, not at all. I did make a superficial run-through and reported a little error and probably should not have, but we are firing on all cylinders in this project. We have been hit by robots downloading data for which we pay egress charges, there have been problems with certification processes for servers, and we are renovating infrastructure that nobody really sees. We are glad to interoperate with Seurat and want to ensure that all opportunities for increasing user satisfaction with Bioconductor packages (such as yours may become) are taken. I am no expert on spatial data or on Seurat but I need to know more and will take this opportunity to work more closely with your package, and I will take over the review process. @HelenaLC (https://github.com/HelenaLC) has done a great job with the review and your technical responses are appreciated. I do regret some of the rhetoric and bellicosity and hope that this can be put behind us. I will write more within a week. If there are particular reasons for seeking a rapid decision please make them known, you may email me directly if this is a concern. — Reply to this email directly, view it on GitHub (https://github.com/Bioconductor/Contributions/issues/3452#issuecomment-2328948725), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AL4ZXLCMYOFUMPKUHKSUFPLZU37MZAVCNFSM6AAAAABIYAYED6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRYHE2DQNZSGU). You are receiving this because you were mentioned.Message ID: @.***>

bioc-issue-bot commented 3 weeks ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

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 Single Package Builder: Linux (Ubuntu 22.04.3 LTS): CatsCradle_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/CatsCradle to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 3 weeks ago

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

bioc-issue-bot commented 3 weeks ago

Dear Package contributor,

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

Your package has been built on the Bioconductor Single Package Builder.

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.

The following are build products from R CMD build on the Single Package Builder: ERROR before build products produced.

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/CatsCradle to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 3 weeks ago

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