Contributions
Contributions copied to clipboard
Bioconductor
CARDspa
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]'
-
[x] I understand that by submitting my package to Bioconductor, the package source and all review commentary are visible to the general public.
-
[x] I have read the Bioconductor Package Submission instructions. My package is consistent with the Bioconductor Package Guidelines.
-
[x] I understand Bioconductor Package Naming Policy and acknowledge Bioconductor may retain use of package name.
-
[x] 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.
-
[x] My package addresses statistical or bioinformatic issues related to the analysis and comprehension of high throughput genomic data.
-
[x] 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.
-
[x] 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:
- [x] The 'devel' branch for new packages and features.
- [x] The stable 'release' branch, made available every six months, for bug fixes.
- [x] 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.
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 = "[email protected]",
role = "aut"),
person(given = "Jing",
family = "Fu",
email = "[email protected]",
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
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
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
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
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
[email protected]:packages/CARDspa to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.
Received a valid push on git.bioconductor.org; starting a build for commit id: d18fb3946a39689ba798c0a79b4e72c2a4d5c83e
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
[email protected]:packages/CARDspa to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
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!
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
spatialExperimentobject. 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
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.
Received a valid push on git.bioconductor.org; starting a build for commit id: e9990881633efce702a4753905d52bacd5da49c8
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
[email protected]:packages/CARDspa to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Hi @vjcitn and @ococrook ,
Thank you for your valuable feedback and I already uploaded the latest version.
-
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.
-
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
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
Received a valid push on git.bioconductor.org; starting a build for commit id: 4fde25969c5c1bd0915efd139aef88f5d19c9962
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
[email protected]:packages/CARDspa to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
We are updating our builders; I believe this to be an error on our end that we are investigating
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
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
[email protected]:packages/CARDspa to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Received a valid push on git.bioconductor.org; starting a build for commit id: efed8d75a75542f4593436117350982f07ec4c16
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
[email protected]:packages/CARDspa to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Hi @ococrook ,
Thank you for your valuable feedback, I have made all the changes.
- [x] Would benefit from a readme
- [x] Please improve unit testing coverage
- [x] 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
- [x] I wouldn’t use “x” to reference an object please use descriptive input names e.g object, params, SpatialExp, etc
- [x] Please check inputs are reasonable e.g. having a “stopifnot” statement that check the inputs are valid
- [x] Remove code that has been commented out, if necessary to keep please isolate away from the structure of the functioning code
- [x] 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
- [x] Could you move classes to AllClasses.R - it took me a while to locate them.
- [x] 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
- [x] I generally found the code difficult to follow as there are a number of standard coding practices aren't kept to I would recommend:
- [x] A) avoid re-using the same variable to have different meanings in the same chunk of code
- [x] B) avoid complex mutil-logical statements in the same line e.g. one-line - one -purpose would be benifical
- [x] 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
- [x] 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
Hi @JINGFU9912
Thanks for the changes - though I dont seem them reflected in your github repo? Did you forget to push?
Best, Olly
Great - I was confused at first. This is much improved thank you! Could you include a NEWS.md file to keep track of changes between version and then bump the version number to 0.99.5 in the description and NEWS file. You can see a NEWS.md file example here: https://github.com/ococrook/bandle/blob/main/NEWS.md
For 0.99.5 you can write bioc ready, and 0.99.4 summarise the changes you made. Youll need to keep the updates as changes are made to the package with version numbers.
Received a valid push on git.bioconductor.org; starting a build for commit id: 31984e0712943e92bd19b98d696fc46ecc2b8a0a