CARDspa

[JINGFU9912]

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

• Repository: https://github.com/YMa-lab/CARDspa

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 Bioconductor Package Naming Policy and acknowledge
  Bioconductor may retain use of package name.

• 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 Bioconductor code of conduct and
  agree to abide by it.

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 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]

Hi @JINGFU9912

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: CARDspa
Title: Spatially Informed Cell Type Deconvolution for Spatial Transcriptomics
Version: 0.99.0
Date: 2024-06-25
Authors@R: 
    c(person(given = "Ying", 
    family = "Ma", 
    email = "ying_ma@brown.edu",
    role = "aut"),
    person(given = "Jing", 
    family = "Fu", 
    email = "jing_fu@brown.edu",
    role = "cre"))
Description: CARD is a reference-based deconvolution method that estimates cell 
    type composition in spatial transcriptomics based on cell type specific 
    expression information obtained from a reference scRNA-seq data. A key 
    feature of CARD is its ability to accommodate spatial correlation in the 
    cell type composition across tissue locations, enabling accurate and 
    spatially informed cell type deconvolution as well as refined spatial map 
    construction. CARD relies on an efficient optimization algorithm for 
    constrained maximum likelihood estimation and is scalable to spatial 
    transcriptomics with tens of thousands of spatial locations and tens of 
    thousands of genes. 
License: GPL-3 + file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.1
Depends: R (>= 4.3.0)
Imports: Rcpp (>= 1.0.7),RcppArmadillo,SingleCellExperiment,
    SummarizedExperiment, methods, MCMCpack, fields, wrMisc, concaveman, 
    sp, dplyr, sf, Matrix, RANN, ggplot2, reshape2, RColorBrewer, 
    scatterpie, grDevices,ggcorrplot, stats, nnls, pbmcapply, RcppML, NMF, 
    spatstat.random, gtools
LazyData: false
biocViews: Spatial, SingleCell, Transcriptomics, Visualization
LinkingTo: 
    Rcpp, RcppArmadillo
Suggests: 
    knitr,
    rmarkdown,
    testthat
VignetteBuilder: knitr
URL: https://github.com/YMa-lab/CARDspa
BugReports: https://github.com/YMa-lab/CARDspa/issues

[lshep]

set.seed should not be used within R functions themselves but called outside the function. If there is some strong reasoning for using within R functions minimally it should be controlled by an argument to the function that the user can control

[JINGFU9912]

Hi @lshep,

Thanks for your comments. I have updated the code to allow users the option to control the seed setting. Here's an explanation of why I use set.seed() in my code:

R/CARD.prop.R (line 268), R/CARD.refFree.R (lines 73 and 86), R/CARD.SCMapping.R (line 64):
The use of set.seed() in these functions is intended to ensure consistent parameter initialization across different runs. Initial conditions can significantly affect the results obtained by the user.

R/visualization.R (line 318):
Here, set.seed() is used to generate a consistent set of colors for visualizations.

Best regards,
Jing

[bioc-issue-bot]

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]

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: CARDspa_0.99.0.tar.gz
Linux (Ubuntu 22.04.3 LTS): CARDspa_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/CARDspa to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

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

[bioc-issue-bot]

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

[bioc-issue-bot]

Dear Package contributor,

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

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

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

[ococrook]

Hi!

Sorry for the delay September has been very busy. Looking through the package it is applied to spatial transcriptomic data. I'm not an expert on this data type but we do have a core principle of re-using Bioconductor infrastructure. Is there any reason that input type cannot be a spatialExperiment object. The code might require a bit of rewriting to get that to work.

Best wishes,
Olly

[JINGFU9912]

Hi!

Sorry for the delay September has been very busy. Looking through the package it is applied to spatial transcriptomic data. I'm not an expert on this data type but we do have a core principle of re-using Bioconductor infrastructure. Is there any reason that input type cannot be a spatialExperiment object. The code might require a bit of rewriting to get that to work.

Best wishes, Olly

Hi Olly,

Thank you for your comments and for taking the time to review CARDspa. I understand the importance of adhering to Bioconductor's principle of reusing existing infrastructure, and I appreciate your suggestion regarding the use of the SpatialExperiment object.

The current design of CARDspa aims to provide flexibility by allowing users to input single-cell and spatial genomics data directly as matrices. This was done to simplify data preparation, especially for users who may not be familiar with SingleCellExperiment or SpatialExperiment.

If we were to switch to SpatialExperiment, it would require users to convert single-cell data into SingleCellExperiment objects as well. Since spatial genomics data typically comes in the form of separate count matrices and spatial coordinates, enforcing the use of SpatialExperiment could add unnecessary complexity for users, who would need to preprocess their data. Furthermore, CARDspa would need to extract the count matrices and coordinates from within the SpatialExperiment object, adding redundant steps.

Best Wishes,
Jing

[vjcitn]

Thanks for your submission.

> getClass("CARD")
Class "CARD" [package "CARDspa"]

Slots:
                                                               
Name:             sc_eset   spatial_countMat   spatial_location
Class:                ANY                ANY         data.frame
                                                               
Name:     Proportion_CARD            project    info_parameters
Class:             matrix          character               list
                                                               
Name:    algorithm_matrix       refined_prop refined_expression
Class:               list             matrix             matrix

You've created an S4 class that shares certain features with SpatialExperiment, but lacks a show method, so printing it leads to screenfuls of raw information.

Our guidelines indicate the importance of supporting interop with existing Bioconductor infrastructure. While you remark that

enforcing the use of SpatialExperiment could add unnecessary complexity for users

I would have a couple of responses. First, we don't enforce use of anything. Reviews and guidelines may nudge
developers in certain directions. Second, I don't know what "unnecessary complexity" is, but printing a CARD object
is currently making the user confront some serious complexity. Third, in our view and in the view of contributing developers, avoiding SpatialExperiment and derivates produces more complexity than using it. This looks like a great package and I hope it gets wide use. If you don't want to use Bioconductor classes, that's fine, if you contribute it to CRAN, Bioconductor packages can import it and make use of it. If that's the direction you wish to pursue, please close the issue. If you wish to receive a review at Bioconductor, please do implement interop with existing classes.

[bioc-issue-bot]

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

[bioc-issue-bot]

Dear Package contributor,

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

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

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

[JINGFU9912]

Hi @vjcitn and @ococrook ,

Thank you for your valuable feedback and I already uploaded the latest version.

1. Based on your suggestion, I have added a show method. Now, when users print the CARD object, they will no longer see excessive raw data output, enhancing usability.

2. I sincerely appreciate your explanation regarding the use of SpatialExperiment and Bioconductor classes. I understand and agree with your concerns, and I believe this approach better promotes consistency within the Bioconductor ecosystem. Thus, I have modified the code to effectively interact with SpatialExperiment and SingleCellExperiment objects. Additionally, I have added a vignette to demonstrate this functionality, ensuring clarity and ease of use.

Please let me know if there are any further comments or suggestions. Thanks again!

Best,
Jing

[ococrook]

Hi Jing

Thanks for the changes, I've been through the code and have some suggestions for improvement::

• Would benefit from a readme
• Please improve unit testing coverage
• Check notes in the build - some are very easy to fix, in particular the sapply note, and the use of the colon to generate sequences e.g. use seq functions instead
• I wouldn’t use “x” to reference an object please use descriptive input names e.g object, params, SpatialExp, etc
• Please check inputs are reasonable e.g. having a “stopifnot” statement that check the inputs are valid
• Remove code that has been commented out, if necessary to keep please isolate away from the structure of the functioning code
• Your parallelization approach breaks on windows, could you allow users to specify the backend that will work for them or make sure this won’t be platform dependent
• Could you move classes to AllClasses.R - it took me a while to locate them.
• I’m not sure whether the “SpatialExperiment” object conversion to CARD is exactly what was intended by interoperability. Yes, your functions will work but would it not be better just to use these object straight-up and use the CARD object behind the scenes? Would appreciate a second opinion from @vjcitn
• I generally found the code difficult to follow as there are a number of standard coding practices aren't kept to I would recommend:
  A) avoid re-using the same variable to have different meanings in the same chunk of code
  B) avoid complex mutil-logical statements in the same line e.g. one-line - one -purpose would be benifical
  C) Employ better use of white-space. Try to connect logical pieces of code together with a prevailing comment e.g. storage, statistical computations, filter etc
  D) You switch styles between using ".", "_" and camelCase. In general use of capital letters is inconsistent, which makes it hard to follow. The most important thing is to be consistent but would suggest having a look the the Bioconductor style guide suggestions

[bioc-issue-bot]

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

[bioc-issue-bot]

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

[lshep]

We are updating our builders; I believe this to be an error on our end that we are investigating

[JINGFU9912]

Hi @lshep,

Thank you for letting me know. I was thinking how to address this error, so your clarification is much appreciated. Please let me know if there is anything I need to do on my end in the meantime.

-Jing

[lshep]

I will kick off a new build for you once we have completed updates

[JINGFU9912]

Thank you so much @lshep

[bioc-issue-bot]

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

[bioc-issue-bot]

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

[bioc-issue-bot]

Dear Package contributor,

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

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

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

[JINGFU9912]

Hi @ococrook ,

Thank you for your valuable feedback, I have made all the changes.

• Would benefit from a readme
• Please improve unit testing coverage
• Check notes in the build - some are very easy to fix, in particular the sapply note, and the use of the colon to generate sequences e.g. use seq functions instead
• I wouldn’t use “x” to reference an object please use descriptive input names e.g object, params, SpatialExp, etc
• Please check inputs are reasonable e.g. having a “stopifnot” statement that check the inputs are valid
• Remove code that has been commented out, if necessary to keep please isolate away from the structure of the functioning code
• Your parallelization approach breaks on windows, could you allow users to specify the backend that will work for them or make sure this won’t be platform dependent
• Could you move classes to AllClasses.R - it took me a while to locate them.
• I’m not sure whether the “SpatialExperiment” object conversion to CARD is exactly what was intended by interoperability. Yes, your functions will work but would it not be better just to use these object straight-up and use the CARD object behind the scenes? Would appreciate a second opinion from @vjcitn
• I generally found the code difficult to follow as there are a number of standard coding practices aren't kept to I would recommend:
• A) avoid re-using the same variable to have different meanings in the same chunk of code
• B) avoid complex mutil-logical statements in the same line e.g. one-line - one -purpose would be benifical
• C) Employ better use of white-space. Try to connect logical pieces of code together with a prevailing comment e.g. storage, statistical computations, filter etc
• D) You switch styles between using ".", "_" and camelCase. In general use of capital letters is inconsistent, which makes it hard to follow. The most important thing is to be consistent but would suggest having a look the the Bioconductor style guide suggestions

---

I have made the following changes based on your suggestions:

-Added a README file.
-Added unit tests for all functions using testthat.
-Addressed all resolvable notes from the build report.
-Renamed variables to be more meaningful.
-Implemented input validation with stopifnot checks.
-Removed commented-out code.
-Used BiocParallel to ensure parallelization works on Windows.
-Moved all class definitions to AllClasses.R.
-Updated functions so that the CARD object is used internally, but the final output is a SpatialExperiment object for better user understanding.
-Refactored code to improve readability and adhere to a consistent style, following Bioconductor guidelines.

Thank you again for your suggestions.

-Jing