SpaNorm

[bhuvad]

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

• Repository: https://github.com/bhuvad/SpaNorm

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 @bhuvad

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: SpaNorm
Title: Spatially-aware normalisation for spatial transcriptomics data
Version: 0.99.0
Authors@R: c(
  person(given = "Agus",
        family = "Salim",
        role = c("aut"),
        email = "salim.a@unimelb.edu.au",
        comment = c(ORCID = "0000-0003-3999-7701")),
  person(given = "Dharmesh D.",
         family = "Bhuva",
         role = c("aut", "cre"),
         email = "dharmesh.bhuva@adelaide.edu.au",
         comment = c(ORCID = "0000-0002-6398-9157"))
  )
Description: This package implements the spatially aware library size normalisation algorithm, SpaNorm. SpaNorm normalises out library size effects while retaining biology through the modelling of smooth functions for each effect. Normalisation is performed in a gene- and cell-/spot- specific manner, yielding library size adjusted data.
License: GPL (>= 3)
Encoding: UTF-8
LazyDataCompression: bzip2
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.1
biocViews: Software, GeneExpression, Transcriptomics, Spatial
Depends:
  R (>= 4.4)
Imports:
    edgeR,
    Matrix,
    matrixStats,
    methods,
    scran,
    Seurat,
    SeuratObject,
    SingleCellExperiment,
    SpatialExperiment,
    stats,
    SummarizedExperiment,
    S4Vectors,
    utils
Suggests:
    testthat (>= 3.0.0),
    knitr,
    rmarkdown,
    prettydoc,
    pkgdown,
    covr,
    BiocStyle,
    ggplot2,
    patchwork
URL: https://bhuvad.github.io/SpaNorm
BugReports: https://github.com/bhuvad/SpaNorm/issues
VignetteBuilder: knitr
Config/testthat/edition: 3
LazyData: true

[vjcitn]

Nice package, I had a preliminary look. What is the principle of subsampling with fraction sample.p in SpaNorm? Is it a purely random sample of spots, or is there stratification, perhaps by region and/or sequencing depth?

[bhuvad]

Hi Vince,

At the moment it is purely random and serves the purpose of evenly sampling the tissue region to obtain an unbiased approximation across space. However, we may provide other sampling strategies to sample dense regions more than sparse regions. The code in the package will easily allow such an extension as we did think of this during the design phase. If you think of examples to motivate such an approach over an unbiased sampling across space, please do share them so that we can prioritise this among our other planned works.

Cheers,
Dharmesh

[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.

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): SpaNorm_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/SpaNorm 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.

[lshep]

I apologize for the delay. I should have a review for you in the next few days.

[bhuvad]

I understand you will have a pretty intense workload right now so all good Lori.

[lshep]

Thank you for your understanding. Please find comments below:

• We normally like non essential files like pkgdown and Dockerfiles, and
  spaNorm.Rproj not to be included directly in the branch on
  Bioconductor. Please remove. You could readd to your repo but .gitignore so
  they aren't checked into our branch. .gitignore's are normally in the top
  level and not in a subdirectory (vignette/.gitignore)

• Consider adding a NEWS file, so your package news will be included in
  Bioconductor release announcements

• NAMESPACE/DESCRIPTION consistency. There are alot of packages listed as
  imports in your Description but have no import in your namespace?

• Consider fixing the coding style notes in the build report

• When I try to run your code in a compiled vignette I get the following
  ERROR

> p_region = spatialCoords(HumanDLPFC) |>
  as.data.frame() |>
  cbind("Region" = HumanDLPFC$AnnotatedCluster) |>
  ggplot(aes(pxl_col_in_fullres, pxl_row_in_fullres, colour = Region)) +
  geom_point() +
  scale_colour_brewer(palette = "Paired", guide = guide_legend(override.aes = list(shape = 15, size = 5))) +
  labs(title = "Region") +
  custom_theme()
p_region
Error in custom_theme() : could not find function "custom_theme"

and then

> logcounts(HumanDLPFC) = log2(counts(HumanDLPFC) + 1)
p_counts = plotGeneExpression(HumanDLPFC, "MOBP") +
  ggtitle("Counts")
p_region + p_counts
Error in plotGeneExpression(HumanDLPFC, "MOBP") : 
  could not find function "plotGeneExpression"

You seem to have hidden code from the user that is necessary for us to reproduce
your vignette. These should probably be included as functions in your package so
the user doens't need to manually input them.

• There are also a few typos, spell check might be beneficial

[bioc-issue-bot]

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

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

[bhuvad]

Hi Lori,

Thank you for taking the time to review my package and for providing such useful feedback to improve the code quality. As most members of the community will agree, we appreciate the time and effort the core team puts into maintaining and enhancing the quality of Bioconductor. I have taken your suggestions on board and have made changes to the package as enumerated below.

• We normally like non essential files like pkgdown and Dockerfiles, and spaNorm.Rproj not to be included directly in the branch on Bioconductor. Please remove. You could readd to your repo but .gitignore so they aren't checked into our branch. .gitignore's are normally in the top level and not in a subdirectory (vignette/.gitignore)

I have removed the redundant .gitignore file as well as the Rproj filewhich serves no purpose. However, the pkgdown, Dockerfile, and devcontainer files are important to retain on our github branch and will therefore also be passed on to the devel branch for Bioconductor. These files are used to create a reproducible development environment for any developer willing to contribute to this project in the future. If I temporarily add them to the .gitignore, push, and re-add them, I would have to make sure that this is done each time prior to an update being submitted to the devel branch. These files are already in the .Rbuildignore so will not be an issue elsewhere and they are very lightweight as well. As such, would it be ok to leave these on the repo due to its value to developers?

• Consider adding a NEWS file, so your package news will be included in Bioconductor release announcements.

I have added the file as requested

• NAMESPACE/DESCRIPTION consistency. There are alot of packages listed as imports in your Description but have no import in your namespace?

Most of these packages are used through explicit function calls (examples enumerated for each below) and will therefore not be in the NAMESPACE file.

1. edgeR::estimateDisp
2. Matrix::colSums
3. matrixStats::colMedians
4. scran::quickCluster
5. SeuratObject::LayerData
6. SingleCellExperiment::sizeFactors
7. SpatialExperiment::spatialCoords
8. SummarizedExperiment::assay
9. S4Vectors::metadata

• Consider fixing the coding style notes in the build report

I have fixed the following notes:

1. Switched LazyData to false in the DESCRIPTION file.
2. Added CITATION file.
3. Fixed issue with global variables/functions W and str. The former was a typo (should be Wsub) that has been fixed and the latter was a missing NAMESPACE definition (which I use the explicit call for, i.e., utils::str).
4. Moved Seurat from imports to suggests as it is currently only used for tests. Future functionality may require it at which point I will move it back.
5. Added CellBiology to biocViews for this package as it is appropriate.
6. Changed 1:... to `seq_len(...).
7. Changed dontrun to donttest.
8. Majority of the long line issues are from documentation and vignettes. The rest cannot be helped as these are mathematical operations that are difficult to read if broken into multiple lines.
9. The long functions (>50 lines) cannot be modularised further in any meaningful way. Where this was possible, I had done that.

• When I try to run your code in a compiled vignette I get the following ERROR (...). You seem to have hidden code from the user that is necessary for us to reproduce your vignette. These should probably be included as functions in your package so the user doens't need to manually input them.

Thank you for bringing this into my attention. I did get a similar query from a user, and I realise now that users are likely to reproduce what they see in the report rather than look at the vignette code. This is what produces the error. I have now added a new function called plotSpatial() to the package that is used in the vignettes. This function is a lot more powerful and can be used to plot gene expression, cell/spot annotation, or reduced dimensions. All code is now visible and reproducible from the vignettes.

• There are also a few typos, spell check might be beneficial

I have performed a spell check on the package and have fixed up spelling errors in the vignette and documentation.

Please let me know if you need me to make any other changes to the package.

Cheers,
Dharmesh

[lshep]

Note: The dockerfile will be out-of-date unless updated each release in its current state; if the intention is to have this as a developer tools it might be worth using the devel docker image rather than tie to a specific release.
Would we be able to meet half way with the Dockerfile and at least move into a inst/scripts/ folder rather than the top level directory?

[bioc-issue-bot]

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

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

[bhuvad]

Those are really good suggestions. I have swapped out the container used. I thought I was using devel but might have forgotten to swap it so thanks for picking that up. I also figured it will be easier to move it to the .devcontainer folder which hosts the other configurations to build the development environment. This was, all code related to the developer tool sit within this one folder. Please let me know if there are issues with this and I will address it as per your guidance.

[lshep]

Thank you. I think this is a happy medium. Thanks for the changes.

[bioc-issue-bot]

Your package has been accepted. It will be added to the
Bioconductor nightly builds.

Thank you for contributing to Bioconductor!

Reviewers for Bioconductor packages are volunteers from the Bioconductor
community. If you are interested in becoming a Bioconductor package
reviewer, please see Reviewers Expectations.

[lshep]

The default branch of your GitHub repository has been added to Bioconductor's
git repository as branch devel.

To use the git.bioconductor.org repository, we need an 'ssh' key to associate with your github user name. If your GitHub account already has ssh public keys (https://github.com/bhuvad.keys is not empty), then no further steps are required. Otherwise, do the following:

1. Add an SSH key to your github account
2. Submit your SSH key to Bioconductor

See further instructions at

https://bioconductor.org/developers/how-to/git/

for working with this repository. See especially

https://bioconductor.org/developers/how-to/git/new-package-workflow/
https://bioconductor.org/developers/how-to/git/sync-existing-repositories/

to keep your GitHub and Bioconductor repositories in sync.

Your package will be included in the next nigthly 'devel' build (check-out from git at about 6 pm Eastern; build completion around 2pm Eastern the next day) at

https://bioconductor.org/checkResults/

(Builds sometimes fail, so ensure that the date stamps on the main landing page are consistent with the addition of your package). Once the package builds successfully, you package will be available for download in the 'Devel' version of Bioconductor using BiocManager::install("SpaNorm"). The package 'landing page' will be created at

https://bioconductor.org/packages/SpaNorm

If you have any questions, please contact the bioc-devel mailing list (https://stat.ethz.ch/mailman/listinfo/bioc-devel); this issue will not be monitored further.