bmill3r commented 2 years ago

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

Repository: https://github.com/JEFworks-Lab/STdeconvolve

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.

bioc-issue-bot commented 2 years ago

Hi @bmill3r

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: STdeconvolve
Type: Package
Title: Reference-free cell-type deconvolution of multi-cellular spatially resolved transcriptomics data 
Version: 0.99.0
Authors@R:
    c(person(given = "Brendan",
 family = "Miller",
 role = c("aut", "cre"),
 email = "bmille79@jh.edu",
 comment = c(ORCID = "0000-0002-9559-4045")),
 person(given = "Jean",
        family = "Fan",
        role = c("aut"),
        email = "jeanfan@jhu.edu",
        comment = c(ORCID = "0000-0002-0212-5451"))
    )
Description: STdeconvolve uses latent Dirichlet allocation to infer latent cell-type proportions and transcriptional profiles within multi-cellular spatially-resolved pixels from spatial transcriptomics datasets.  
biocViews: Transcriptomics, Visualization, Deconvolution, RNASeq, TopicModels, Spatial
License: GPL-3
Encoding: UTF-8
LazyData: FALSE
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.1.2
Imports:
    topicmodels,
    parallel,
    BiocParallel,
    Matrix,
    ggplot2,
    scatterpie,
    viridis,
    dynamicTreeCut,
    slam,
    mgcv,
    gridExtra,
    KernSmooth,
    stats,
    clue,
    hash,
    liger,
    dplyr,
    reshape2
LinkingTo:
    Rcpp
Depends:
    R (>= 3.6)
Suggests: 
    knitr,
    rmarkdown,
    testthat,
    rcmdcheck,
    gplots
VignetteBuilder:
    knitr,
    rmarkdown

lshep commented 2 years ago

Deconvolution and TopicModels are not valid biocViews terms. Please remove them.

bioc-issue-bot commented 2 years ago

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

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

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

bmill3r commented 2 years ago

Edits to STdeconvolve after viewing the build report:

The following ERRORs have been addressed: 1.

 Checking vignette directory...
    * ERROR: VignetteBuilder listed in DESCRIPTION but not found as
      VignetteEngine in any vignettes:
        rmarkdown

In the DESCRIPTION:

VignetteBuilder:
    knitr,
    rmarkdown

has now been changed to:

VignetteBuilder:
    knitr

2.

* ERROR: At least 80% of man pages documenting exported objects
      must have runnable examples. The following pages do not:
      annotateCellTypesGSEA.Rd, bh.adjust.Rd, cleanCounts.Rd,
  correlationPlot.Rd, darken.Rd, filterTheta.Rd, fitLDA.Rd,
  getBetaTheta.Rd, getCorrMtx.Rd, getOverdispersedGenes.Rd,
  gg_color_hue.Rd, lighten.Rd, lsatPairs.Rd, normalizeCounts.Rd,
  optimalModel.Rd, perplexityPlot.Rd, preprocess.Rd, reduceTheta.Rd,
  restrictCorpus.Rd, scale0_1.Rd, topGenes.Rd, transparentCol.Rd,
  vizAllTopics.Rd, vizGeneCounts.Rd, vizTopic.Rd, where.is.knee.Rd,
  winsorize.Rd

Corrected by adding @examples to the functions intended to be exported. Added @noRd flags to functions that serve as background or helper functions.

3.

* Checking for bioc-devel mailing list subscription...
    * ERROR: Maintainer must subscribe to the bioc-devel mailing list.
      Subscribe here: https://stat.ethz.ch/mailman/listinfo/bioc-devel

I am now subscribed.

4.

* Checking for support site registration...
    * ERROR: Maintainer must register at the support site; visit
      https://support.bioconductor.org/accounts/signup/ .

I am now registered.

In addition, the following WARNINGs have been addressed: 1.

* WARNING: 'Deconvolution' is an invalid BiocViews term.

BiocViews have been edited to contain:

biocViews: Transcriptomics, Visualization, RNASeq, Bayesian, Spatial, Software, GeneExpression

2.

* WARNING: 'TopicModels' is an invalid BiocViews term.

BiocViews have been edited to contain:

biocViews: Transcriptomics, Visualization, RNASeq, Bayesian, Spatial, Software, GeneExpression

3.

* Checking DESCRIPTION/NAMESPACE consistency...
    * WARNING: Import parallel, gridExtra, KernSmooth, dplyr in
      NAMESPACE as well as DESCRIPTION.

These are listed in the DESCRIPTION file but I don't seem them listed in the NAMESPACE. Can you give any insight as to how I should proceed?

4.

* WARNING: Avoid T/F variables; If logical, use TRUE/FALSE (found 3
      times)
        T in R/import.R (line 157, column 69)
        F in R/import.R (line 157, column 61)
        F in R/import.R (line 199, column 77)

This has been corrected.

5.

* WARNING: Remove set.seed usage (found 2 times)
        set.seed() in R/functions.R (line 433, column 5)
        set.seed() in R/functions.R (line 439, column 5)

It is necessary to set a seed to ensure reproducibility. Note that the value of the seed is preset to 0 as default such that it should produce the same result each time the code is run. However, this can be changed if the user desires.

6.

* Checking man page documentation...
    * WARNING: Add non-empty \value sections to the following man
      pages: man/bh.adjust.Rd, man/correlationPlot_2.Rd,
      man/correlationPlot.Rd, man/darken.Rd, man/gg_color_hue.Rd,
      man/lighten.Rd, man/perplexityPlot.Rd, man/restrictCorpus.Rd,
      man/transparentCol.Rd, man/vizAllTopics.Rd, man/vizGeneCounts.Rd,
      man/vizTopic.Rd, man/vizTopicClusters.Rd

Corrected by adding @return to the functions intended to be exported. Added @noRd flags to functions that serve as background or helper functions.

Changes have been pushed as version 0.99.1 of the default branch. EDIT: made one additional small edit to the DESCRIPTION file. Changed the title to uppercase. Also in "Description", replaced package name with "This package ...". Bumped version to 0.99.2

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

bmill3r commented 2 years ago

There appears to be 2 ERRORs in this build report:

* Checking for bioc-devel mailing list subscription...
    * ERROR: Maintainer must subscribe to the bioc-devel mailing list.
      Subscribe here: https://stat.ethz.ch/mailman/listinfo/bioc-devel
* Checking for support site registration...
    Maintainer is registered at support site.
    * ERROR: Maintainer must add package name to Watched Tags on the
      support site; Edit your Support Site User Profile to add Watched
      Tags.

I have added STdeconvolve as a Watched Tag in my profile. I subscribed to the bioc-devel mailing list but perhaps this has not been approved yet?

Thanks, Brendan

lshep commented 2 years ago

I just check the bioc-devel mailing list and there is no bmille79@jh.edu registered. Please try again. There is a confirmation email that gets sent that often ends up in a spam folder. I confirmed the support site tag was added. Ignore this for now and if it persists I can investigate further.

bmill3r commented 2 years ago

Thanks for letting me know. You are correct - the confirmation email was in my junk folder! I believe I should be registered now and will push another build.

Thanks again, Brendan

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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. This link will be 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/STdeconvolve 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 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

bmill3r commented 2 years ago

Hello,

Most recent version push (0.99.4) appears to just have two remaining WARNINGS:

1.

WARNING: Import parallel, gridExtra, KernSmooth, dplyr, methods,
      utils in NAMESPACE as well as DESCRIPTION.

I see these as Imports in my DESCRIPTION file, however, I do not see these listed in my NAMESPACE. Can you give any insight as to how I should proceed in addressing this?

2.

WARNING: Remove set.seed usage (found 2 times)
        set.seed() in R/functions.R (line 453, column 5)
        set.seed() in R/functions.R (line 459, column 5)

Both of these are in function fitLDA(), where it is necessary to set a seed to ensure reproducibility. Note that the value of the seed is preset to 0 as default such that it should produce the same result each time the code is run. However, this can be changed if the user desires.

Thank you for your assistance and time! Brendan

vjcitn commented 2 years ago

1: why is KernSmooth in DESCRIPTION? I don't see it used in code or in vignette. You need to use an @import directive in your roxygen to get it into NAMESPACE but if you don't use it this would not be needed. So prune it out of the DESCRIPTION or use it explicitly.

2: set.seed clobbers the .Random.seed ... we don't want it used in code. Instruct the user in how to initialize the random number generator for reproducibility but don't do it for them.

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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. This link will be 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/STdeconvolve 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 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

Please see the build report for more details. This link will be 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/STdeconvolve to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bmill3r commented 2 years ago

Thank you @vjcitn and the rest of the reviewers for your help.

I have made edits to the roxygen function headers and package Imports in DESCRIPTION. In addition, I have addressed the issues with set.seed(). It looks like the latest build has passed without any ERRORs or WARNINGs.

Thanks again for your time, Brendan

LiNk-NY commented 2 years ago

Hi Brendan, @bmill3r

Thank you for your submission. Please ensure that you are interoperating with Bioconductor infrastructure as stated in the https://contributions.bioconductor.org/bioconductor-package-submissions.html#subintro. Please see the review below.

Best, Marcel

STdeconvolve #2530

Please use existing Bioconductor interfaces to spatial transcriptomics data, e.g. support for SpatialExperiment and use of TENxVisiumData in the vignette.

DESCRIPTION

Update the Description field to be at least three sentences.
Include a BugReports and URL field pointing to at least the appropriate locations on GitHub
Update the R version to the one noted in BiocCheck
Wrap text in the Description field so that it is within 80 character-width.

NAMESPACE

Looks good.

vignettes/

Consider using BiocStyle for the vignette.
Please keep text within the 80 character limit for easier review and arguably maintenance (in vim use gqq).
It would be good to demonstrate the use a 10x Visium dataset (possibly from TENxVisiumData) to demonstrate the functionality and to interface with the SpatialExperiment package.
The vignette(s) in the package should be as extensive as those in https://jef.works/STdeconvolve/ but not less. This means porting the Visium examples with the suggestions above.

R

As mentioned, provide support for SpatialExperiment inputs to your functions.
matrix(unlist(read.table(...))) looks a little awkward, consider using as.matrix on the data.frame.
Remove commented code if not used.
Use the NOT operator (!) to check for FALSE instead of if (x == FALSE).
Include options in the arguments, e.g., type = c("t", "b") and use match.arg in the body of the function.
Do not use ; in R code.
Check class membership using is(x, 'classname')
Minor: is.matrix(x) == TRUE is redundant and should simply be is.matrix(x).
You can use as.integer(verbose) to convert and reduce cyclomatic complexity.

tests

Please include unit tests to make your code more robust.

> covr::package_coverage()
STdeconvolve Coverage: 0.00%
R/functions.R: 0.00%
R/import.R: 0.00%
R/misc.R: 0.00%
R/plot.R: 0.00%

bmill3r commented 2 years ago

Hi Marcel @LiNk-NY,

Thank you so much for the detailed submission review. I have tried to address as much of the comments as possible. Please see below for my responses. A new version bump will be submitted momentarily to trigger a new build (version 0.99.7).

#

[x] Please use existing Bioconductor interfaces to spatial transcriptomics data, e.g. support for SpatialExperiment and use of TENxVisiumData in the vignette.
I have added a new section in the vignette demonstrating how appropriate information from SpatialExperiment objects and by extension TENxVisiumData datasets can be input into STdeconvolve. See section: # SpatialExperiment inputs.

#

Description

[x] Update the Description field to be at least three sentences.

I have extended the Description field as follows:

Description: STdeconvolve as an unsupervised, reference-free approach to infer
latent cell-type proportions and transcriptional profiles within
multi-cellular spatially-resolved pixels from spatial transcriptomics (ST)
datasets. STdeconvolve builds on latent Dirichlet allocation (LDA), a
generative statistical model commonly used in natural language processing
for discovering latent topics in collections of documents. In the context of
natural language processing, given a count matrix of words in documents, LDA
infers the distribution of words for each topic and the distribution of
topics in each document. In the context of ST data, given a count matrix of
gene expression in multi-cellular ST pixels, STdeconvolve applies LDA to
infer the putative transcriptional profile for each cell-type and the
proportional representation of each cell-type in each multi-cellular ST
pixel.

[x] Include a BugReports and URL field pointing to at least the appropriate locations on GitHub

These fields have now been added:

URL: https://jef.works/STdeconvolve/
BugReports: https://github.com/JEFworks-Lab/STdeconvolve/issues

[ ] Update the R version to the one noted in BiocCheck
I have not done this yet as I am concerned that requiring users to update their version of R to 4.2 may be a hurdle many will not want to overcome in order to use this tool. I have several colleagues who are using this tool with older versions of R with success. Is this a required condition for adaption into Bioconductor? Do you have other suggestions of how to proceed?
[x] Wrap text in the Description field so that it is within 80 character-width.
This has been done (albeit manually; I am curious if you know of more automated approaches I can employ in the future?)

#

Namespace

Looks good.

Thank you!

#

vignettes/

[x] Consider using BiocStyle for the vignette.

I have added to the rmarkdown header:

---
title: "STdeconvolve Vignette"
author: "Brendan F. Miller"
output: BiocStyle::html_document
vignette: >
%\VignetteIndexEntry{STdeconvolve Vignette}
%\VignetteEncoding{UTF-8}
%\VignetteEngine{knitr::rmarkdown}
---

and have added to the Description:
```
Suggests: 
knitr,
BiocStyle,
```
[x] Please keep text within the 80 character limit for easier review and arguably maintenance (in vim use gqq).
I have tried to do this where possible. There are a few lines of code with long string variables that extend past this character limit.
[x] It would be good to demonstrate the use a 10x Visium dataset (possibly from TENxVisiumData) to demonstrate the functionality and to interface with the SpatialExperiment package.
I have added a new section in the vignette demonstrating how appropriate information from SpatialExperiment objects and by extension TENxVisiumData datasets can be input into STdeconvolve. See section: # SpatialExperiment inputs.
[x] The vignette(s) in the package should be as extensive as those in https://jef.works/STdeconvolve/ but not less. This means porting the Visium examples with the suggestions above.
The examples from the Visium tutorial have been added. Additionally, I have new code demonstrating how to interact with datasets from TENxVisiumData not previously in the Visium example from https://jef.works/STdeconvolve/. In addition to working with Visium data, the vignette demonstrates data preprocessing, model fitting, visualization, and annotation steps, all part of the full workflow of STdeconvolve.

#

R

[ ] As mentioned, provide support for SpatialExperiment inputs to your functions.

As shown in the vignette, there are just two lines of code needed to extract the required information. Is it necessary to write a wrapper function around these? For example:

## to get the counts matrix from an "se" SpatialExperiment object:
cd <- se@assays@data@listData$counts
## to get the position matrix from an "se" SpatialExperiment object:
pos <- SpatialExperiment::spatialCoords(se)

[x] matrix(unlist(read.table(...))) looks a little awkward, consider using as.matrix on the data.frame.
The line of code that utilized this is actually deprecated in the current usage of STdeconvolve and has been removed.
[x] Remove commented code if not used.
Unused code has been removed.
[x] Use the NOT operator (!) to check for FALSE instead of if (x == FALSE).
Instances of if(x == FALSE) have been replaced using if(!x).
[x] Include options in the arguments, e.g., type = c("t", "b") and use match.arg in the body of the function.
type = c("t", "b") and match.arg has been incorporated into the function getCorrMtx().
[x] Do not use ; in R code.
Instances of ; have been removed.
[ ] Check class membership using is(x, 'classname')
Could you please clarify - would this include all instances of where I test:
```
is.na(x)
is.matrix(x)
is.data.frame(x)
is.numeric(x)
```
etc, ?
[x] Minor: is.matrix(x) == TRUE is redundant and should simply be is.matrix(x).
The redundancy of == TRUE for instances of is.matrix(). is.na(), etc have been removed.
[x] You can use as.integer(verbose) to convert and reduce cyclomatic complexity.
Thank you for the suggestion. This has been incorporated specifically in the function fitLDA()

#

tests

[x] Please include unit tests to make your code more robust.
test_that checks are now utilized. Also, functions in R/misc.R are actually functions that are deprecated in the default usage of STdeconvolve and are not used. Thus, these function are no longer part of the default package branch but are present on the devel branch.

> covr::package_coverage()
STdeconvolve Coverage: 55.41%
R/plot.R: 48.45%
R/import.R: 52.31%
R/functions.R: 60.96%

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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. This link will be 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/STdeconvolve 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 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

bmill3r commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

On most recent build I am getting the following error:

Error: package or namespace load failed for BiocCheck in loadNamespace(j <- i[[1L]], c(lib.loc, .libPaths()), versionCheck = vI[[j]]):
 there is no package called Biobase

I have not needed this package before. Should this be added under Suggests in the DESCRIPTION ?

Thanks, Brendan

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "TIMEOUT". 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. This link will be 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/STdeconvolve 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 years ago

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

bmill3r commented 2 years ago

Hello,

I wanted to check in and see if anyone could provide insight as to why the build report is taking so long to generate. On my end the testthat checks and software build take just a few minutes. I can submit another build with a version bump to try again.

Thanks again for all of your time and help, Brendan

lshep commented 2 years ago

We were having issues with our windows builder. I think I have it remedied and will kick off a manual build for you now. The report should post shortly

lshep commented 2 years ago

Something is preventing the windows builder from completing. I think this is on our end and I'm investigating the matter. In the meantime you can review results of triggered builds here. I hope to have the issue resolved shortly.

LiNk-NY commented 2 years ago

Hi @bmill3r

I have not done this yet as I am concerned that requiring users to update their version of R to 4.2 may be a hurdle many will not want to overcome in order to use this tool. I have several colleagues who are using this tool with older versions of R with success. Is this a required condition for adaption into Bioconductor? Do you have other suggestions of how to proceed?

How can you guarantee a tool works with an older version of R than the one you have developed it with? Once in Bioconductor, the version requirements will be more strict than on GitHub. If you'd like to keep the relaxed version requirement (at your own risk) you can create a separate legacy branch.

This has been done (albeit manually; I am curious if you know of more automated approaches I can employ in the future?)

I like to use vim gqq but there are other tools.

As shown in the vignette, there are just two lines of code needed to extract the required information. Is it necessary to write a wrapper function around these? For example:

## to get the counts matrix from an "se" SpatialExperiment object:
cd <- se@assays@data@listData$counts
## to get the position matrix from an "se" SpatialExperiment object:
pos <- SpatialExperiment::spatialCoords(se)

For the purposes of the vignette, it does not seem you need wrapper function for this operation. Please use the standard interface for the class and avoid encouraging bad practice, i.e., use of @. The following has the same effect for cd:

cd <- assay(se, "counts")

Could you please clarify - would this include all instances of where I test:

?methods::is is best used for S4 type classes. S3 classes / types have either is.<type> or inherits(x, 'classname') functions that you could use.

Please resolve the error in the output of the report. Particularly, registering for the bioc-devel mailing list and the support site. Make sure to resolve this error as well.

* Checking vignette directory...
    * ERROR: VignetteBuilder listed in DESCRIPTION but not found as
      VignetteEngine in any vignettes:
        rmarkdown

Remember to run BiocCheck on your package and resolve the issues there as well.

Best, Marcel

bioc-issue-bot commented 2 years ago

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

bmill3r commented 2 years ago

Hi @LiNk-NY,

Thank you so much for the helpful comments. I have updated the R version dependency on the default branch to >=4.1 which is what is currently installed on my local machine, interface with the SpatialExperiment object to extract the counts matrix in the vignette, and the VignetteBuilder ERROR appears to be resolved now. Class membership checks are done on S3 objects and so this is done with is.<type> where needed.

R CMD check and BiocCheck on my end do not return any errors or warnings. Pushing a new build shortly for version 0.99.10.

Thanks again for all of your help, Brendan

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

Please see the build report for more details. This link will be 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/STdeconvolve to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

LiNk-NY commented 2 years ago

@bmill3r The changes look good.

Please do not check in the README.html file. This is not needed.

Best, Marcel

bioc-issue-bot commented 2 years ago

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

bmill3r commented 2 years ago

Hi @LiNk-NY,

Thanks again for all of your help. I have removed the README.html and version bumped to 0.99.11.

Brendan

bioc-issue-bot commented 2 years ago

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.

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "TIMEOUT". 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. This link will be 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/STdeconvolve to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

LiNk-NY commented 2 years ago

Hi @bmill3r Please make sure that the package is not close to the timeout limit. We recommend using small datasets to demonstrate the functionality. Best, Marcel

bmill3r commented 2 years ago

Hi @LiNk-NY

I have reduced the time to run the examples and tests so hopefully it should run through the checks faster. I apologize for the back and forth. On my local machine and pushes to GitHub everything finishes in less than 14 minutes so it's hard to tell why I get timeouts with my pushes to Bioconductor. Pushing build version 0.99.12

bioc-issue-bot commented 2 years ago

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

bioc-issue-bot commented 2 years ago

Dear Package contributor,

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

Your package has been built on Linux, Mac, and Windows.

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

Please see the build report for more details. This link will be 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/STdeconvolve to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

LiNk-NY commented 2 years ago

Hi @bmill3r

Thank you for making those changes and sticking with it. Your package has been accepted. Thank you for your contribution.

Best regards, Marcel

bioc-issue-bot commented 2 years ago

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.

bmill3r commented 2 years ago

Hi @LiNk-NY and Reviewers,

Thanks so much taking the time to review this submission. This process has overall greatly improved the package and I have learned a ton along the way. I really appreciate it.

Brendan

lshep commented 2 years ago

The master branch of your GitHub repository has been added to Bioconductor's git repository.

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/bmill3r.keys is not empty), then no further steps are required. Otherwise, do the following:

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("STdeconvolve"). The package 'landing page' will be created at

https://bioconductor.org/packages/STdeconvolve

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.

Bioconductor / Contributions