Contributions icon indicating copy to clipboard operation
Contributions copied to clipboard

staRgate

Open leejasme opened this issue 1 year ago • 26 comments

Hello Bioconductor team, I am submitting my R package {staRgate} in consideration for Bioconductor release.

Thank you for your time and consideration!

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

  • Repository: https://github.com/leejasme/staRgate

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.

leejasme avatar Jul 11 '24 20:07 leejasme

Hi @leejasme

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: staRgate
Title: Automated gating pipeline for flow cytometry analysis to characterize 
    the lineage, differentiation, and functional states of T-cells
Version: 0.99.0
Authors@R: 
    person("Jasme", "Lee", email = "[email protected]", role = c("aut", "cre"),
    comment = c(ORCID = "0009-0006-4492-4872"))
Description: An R-based automated gating pipeline for flow cytometry data designed 
    to mimic the manual gating strategy of defining flow biomarker positive populations 
    relative to a unimodal background population to include cells with varying 
    intensities of marker expression. The pipeline’s main feature is a flexible 
    density-based gating strategy capable of capturing varying scenarios based on 
    marker expression patterns to analyze a 29-marker flow panel that characterizes 
    T-cell lineage, differentiation, and functional states.
License: MIT + file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.1
biocViews: FlowCytometry, Preprocessing, ImmunoOncology
Imports: 
    dplyr,
    janitor,
    magrittr,
    purrr,
    rlang,
    stringr,
    tidyr, 
    flowCore, 
    flowWorkspace,
    glue, 
    tibble
Suggests: 
    flowAI,
    ggplot2,
    gt,
    knitr,
    openCyto,
    ggcyto,
    rmarkdown,
    data.table,
    here,
    testthat (>= 3.0.0)
VignetteBuilder: knitr
BugReports: https://github.com/leejasme/staRgate/issues
URL: https://leejasme.github.io/staRgate/, https://github.com/leejasme/staRgate
Config/testthat/edition: 3

bioc-issue-bot avatar Jul 11 '24 20:07 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 avatar Jul 18 '24 13:07 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: staRgate_0.99.0.tar.gz Linux (Ubuntu 22.04.3 LTS): staRgate_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/staRgate to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Jul 18 '24 13:07 bioc-issue-bot

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

bioc-issue-bot avatar Jul 23 '24 13:07 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: staRgate_0.99.1.tar.gz Linux (Ubuntu 22.04.3 LTS): staRgate_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/staRgate to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Jul 23 '24 13:07 bioc-issue-bot

Hello!

The latest warning that was flagged states: " * WARNING: Package listed as VignetteEngine or VignetteBuilder but not currently Suggested."

I only use knitr for the vignette and have included in the Suggests field. When running the checks on my local machine, I do not get this warning and cannot reproduce.

It looks like this might be a false positive based on a few other packages currently undergoing review (#3379 and #3451)

What is the best course of action for moving this forward?

Thank you!

leejasme avatar Jul 24 '24 23:07 leejasme

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 avatar Aug 05 '24 14:08 bioc-issue-bot

@LiNk-NY can you also investigate the reported false positive in the BiocCheck?

lshep avatar Aug 05 '24 14:08 lshep

Hi @lshep The issue was resolved two weeks ago : https://github.com/Bioconductor/BiocCheck/commit/481d8d1fa9c5ac32672d1119a058b8add2a933ed

@leejasme Please bump the version for a more recent run BiocCheck.

LiNk-NY avatar Aug 05 '24 14:08 LiNk-NY

Great thank you! Will do

leejasme avatar Aug 05 '24 19:08 leejasme

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

bioc-issue-bot avatar Aug 05 '24 20:08 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: macOS 12.7.1 Monterey: staRgate_0.99.2.tar.gz Linux (Ubuntu 22.04.3 LTS): staRgate_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/staRgate to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Aug 05 '24 20:08 bioc-issue-bot

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

bioc-issue-bot avatar Aug 05 '24 21:08 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): staRgate_0.99.3.tar.gz macOS 12.7.1 Monterey: staRgate_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/staRgate to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Aug 05 '24 21:08 bioc-issue-bot

Hi @LiNk-NY, thank you for the initial review!

I wanted to clarify on the first point as I am not familiar with the SummarizedExperiment and stageR class. Do you mean entirely for the pipeline whether it would make sense to change to these classes, or only for the subsequent part where I pull the intensity matrix from the GatingSet?

I will look more closely into each of the comments and update accordingly. Thank you!

leejasme avatar Aug 09 '24 18:08 leejasme

Hi @leejasme

Apologies for the confusion. I copied the wrong review.

Please see the review below. Let me know if you have any questions. Best regards, Marcel


staRgate #3485

DESCRIPTION

  • Looks good.
  • Consider avoiding the magrittr dependency in favor of the native pipe (|>).

NAMESPACE

  • Consider using the native pipe.
  • Looks good.

vignettes

  • Consider using BiocStyle::html_document as the output function.
  • Consider wrapping the lines to a limit of 80 character width. This makes it easier to review and, IMO, maintian.
  • Remove version specific code from the Installation section. You do not want to update this code at every release.
  • Remove GitHub installation instructions.
  • Remove installation instructions for specific packages and only keep the library("package") calls instead. Import'ed and Suggest'ed packages are usually installed with your package.
  • Use r Biocpkg("flowCore"} etc. for referring to Bioconductor or r CRANpkg("openCyto") for CRAN packages
  • Consider hiding (echo = FALSE) the theme_set code related to plotting.
  • Optional. Please encourage good practices and use <- instead of = for assignment.
  • Avoid using @ to access slots and create an accessor function (e.g. in chk_cm@spillover).
  • Avoid too much conditional code as it complicates the workflow for the user, e.g., when resolving inconsistencies in naming conventions in the Import FCS section
  • Consider reducing the amount of repetitive code in the vignette. Please make it easy for the user to use your software (see Extract intensity matrix section).

R

  • Keep move the roxygen block(s) above the function and not in the body (see getBiexpTransformGS).
  • Is the standard that all input files be csv? Perhaps the inputs to these functions can be matrices instead. What if names differ, is your code robust enough to handle column name changes?
  • Consider reducing the cyclomatic complexity in the code by reducing the number of "check inputs" and consider using an S4 class with validity checks.
  • Avoid nested ifelse and use case_when with mutate.
  • Minor: Consider using the shorter is.data.frame(), is.numeric() functions instead of inherits(...)

tests

Please increase the package coverage in R/getDensityGates.R:

> covr::package_coverage(type="all")
staRgate Coverage: 81.35%
R/getDensityGates.R: 52.63%
R/getPerc.R: 74.40%
R/getGatedDat.R: 84.38%
R/internal.R: 98.41%
R/getBiexpTransformGS.R: 100.00%
R/getCompGS.R: 100.00%

LiNk-NY avatar Aug 13 '24 18:08 LiNk-NY

Thank you!! I will work on updating!

Is there a timeline for when I would need to push updates before this issue would automatically close etc?

leejasme avatar Aug 14 '24 01:08 leejasme

Hi @leejasme We usually expect some activity within two weeks of the review. If you need more time, let us know. Best regards, Marcel

LiNk-NY avatar Aug 14 '24 16:08 LiNk-NY

This issue is being closed because there has been no progress for an extended period of time. You may reopen the issue when you have the time to actively participate in the review / submission process. Please also keep in mind that a package accepted to Bioconductor requires a commitment on your part to ongoing maintenance.

Thank you for your interest in Bioconductor.

bioc-issue-bot avatar Jan 14 '25 19:01 bioc-issue-bot

Hello Bioconductor team, I would like to reopen this issue to submit an updated version of the package for consideration following the helpful first review! Is it sufficent to reopen this issue and bump the version number? Please let me know if I should do anything else to put this back in the queue. Thank you so much!

leejasme avatar Dec 09 '25 15:12 leejasme

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 avatar Dec 09 '25 15:12 bioc-issue-bot

Dear @leejasme ,

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

bioc-issue-bot avatar Dec 09 '25 15:12 bioc-issue-bot

yes you should bump the version to trigger a new build report and comment on any of the items previously identified in the review process.

lshep avatar Dec 09 '25 15:12 lshep

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

bioc-issue-bot avatar Dec 09 '25 15:12 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.3 LTS): staRgate_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/staRgate to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Dec 09 '25 15:12 bioc-issue-bot

Thank you! I just pushed a new version bump. Below are comments RE the first review

DESCRIPTION AND NAMESPACE

  • Updated all to native pipes

vignette

  • Updated to using BiocStyle::html_document, removed installation instructions,
  • Whenever possible wrapped code lines to 80 char width except for strings or comments
  • Use r Biocpkg() and r CRANpkg to reference packages
  • Avoid using @ to access slots and create an accessor function (to the best of my knowledge the package did not have an accessor function for getting the spillover)
  • Cleaned up some repetitive and conditional code to make it easier for user to read and use.

R

  • For inputs, currently the expectation is the input will be csv files instead of matrices but this may be updated in the future to allow both types of inputs for more flexibility.
  • Updated the method for checking inputs into one wrapping function applied across multiple functions
  • updated nested ifelse code with case_when when possible
  • updated to use is.data.frame() etc. functions instead of inherits()

Tests

  • Updated to include more testing to increase to at least 90% coverage

leejasme avatar Dec 09 '25 16:12 leejasme

Hi @leejasme

Thank you for your updates.

After taking a quick look, the package seems to be in good shape. There appears to be some redundancy especially in rlang::* condition signals. For example, instances where "error" and "warning" keywords are in the message of the condition:

rlang::abort("Error: This is an error message")
#' Error:
#' ! Error: This is an error message
#' Run `rlang::last_trace()` to see where the error occurred.

This extra "Error" in the message is redundant since rlang::abort and others already flag the condition in the first line with Error:, as seen above.

Please make sure that you are resolving as many NOTEs as possible in R CMD check and BiocCheck.

-MR

LiNk-NY avatar Jan 08 '26 20:01 LiNk-NY

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 avatar Jan 08 '26 20:01 bioc-issue-bot

Thank you for the review and excited that it is now part of Bioconductor!!

Thanks for pointing out about the messages, will update that!

leejasme avatar Jan 08 '26 22:01 leejasme

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

https://bioconductor.org/packages/staRgate

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.

lshep avatar Jan 12 '26 15:01 lshep