Submit CytoPipeline package (2nd attempt)

[phauchamps]

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

• Repository: https://github.com/UCLouvain-CBIO/CytoPipeline

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]

Hi @phauchamps

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: CytoPipeline
Title: Automation and visualization of flow cytometry data analysis pipelines
Version: 0.99.0
Authors@R:
    c(person(given = "Philippe",
   family = "Hauchamps",
   role = c("aut", "cre"),
   email = "philippe.hauchamps@uclouvain.be",
   comment = c(ORCID = "0000-0003-2865-1852")),
      person(given = "Laurent", family = "Gatto",
   email = "laurent.gatto@uclouvain.be",
   role = "aut",
   comment = c(ORCID = "0000-0002-1520-2268")),
      person(given = "Dan",
   family = "Lin",
   role = "ctb",
   email = "dan.8.lin@gsk.com"))
Description: This package provides support for automation and visualization of
 flow cytometry data analysis pipelines. In the current state, the package
 focuses on the preprocessing and quality control part. The framework is based
 on two main S4 classes, i.e. CytoPipeline and CytoProcessingStep. The pipeline
 steps are linked to corresponding R functions - that are either provided in
 the CytoPipeline package itself, or exported from a third party package,
 or coded by the user her/himself. The processing steps need to be specified
 centrally and explicitly using either a json input file or through step by step
 creation of a CytoPipeline object with dedicated methods. After having run the
 pipeline, obtained results at all steps can be retrieved and visualized thanks
 to file caching (the running facility uses a BiocFileCache implementation).
 The package provides also specific visualization tools like
 pipeline workflow summary display, and 1D/2D comparison plots of obtained
 flowFrames at various steps of the pipeline.
License: GPL (>=3) | file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.0
LazyData: true
LazyDataCompression: gzip
BugReports: https://github.com/UCLouvain-CBIO/CytoPipeline/issues
URL: https://uclouvain-cbio.github.io/CytoPipeline
biocViews: FlowCytometry, Preprocessing, QualityControl, WorkflowStep,
 ImmunoOncology, Software, Visualization
Collate:
    'CytoPipeline-functions.R'
    'CytoPipeline.R'
    'CytoProcessingStep.R'
    'CytoProcessingStepImplementations.R'
    'data.R'
    'gating.R'
    'utils.R'
    'ggplots.R'
Depends:
    R (>= 4.2),
    flowCore
Imports:
    methods,
    stats,
    utils,
    withr,
    ggplot2,
    ggcyto,
    BiocFileCache,
    BiocParallel,
    openCyto,
    PeacoQC,
    flowAI,
    flowCut,
    flowClean,
    flowClust,
    flowStats,
    diagram,
    jsonlite,
    scales
Suggests:
    testthat (>= 3.0.0),
    vdiffr,
    diffviewer,
    knitr,
    rmarkdown,
    BiocStyle
VignetteBuilder: knitr
Config/testthat/edition: 3

[vjcitn]

Quitting from lines 363-365 (CytoPipeline.Rmd) 
Error: processing vignette 'CytoPipeline.Rmd' failed with diagnostics:
'gate_tail' is not an exported object from 'namespace:openCyto'
--- failed re-building ‘CytoPipeline.Rmd’

SUMMARY: processing the following file failed:

Please attend to this.

[phauchamps]

Hi Vince,

The problem above was due to a change in the openCyto package, from which the previously exported gate_tail() function was removed, without any prior notice :-( Therefore I removed this dependency from the CytoPipeline package, and replaced it by another function.

However, there is still another dependency that prevents my CytoPipeline package from checking correctly, in the runExamples and unit test section. This is due to a pending issue in another package from which CytoPipeline depends: ggcyto::scale_x_logicle() which seems to not converge anymore in the recent Bioc repo commits, and creates build errors in the current Bioc devel build (see http://bioconductor.org/checkResults/release/bioc-LATEST/ggcyto/nebbiolo2-buildsrc.html )

I would have two questions for you:

1. in your opinion, shall I wait until this build error for ggcyto is solved (it is currently impacting both devel AND release builds in all Bioc technical environments ) or raise myself an issue to ggcyto package's maintainer ?
2. I have tried to commit my code changes to github repo, but I think that my package has not been incorporated yet in the repo. Could you confirm ? Will it be done when the current issues moves to '2. Review in progress' status ?

Thanks a lot,

Philippe

[lshep]

Jumping in here: I would say feel free to reach out to the maintainer of ggcyto yourself. We will as well but it is always helpful and sometime knowing someone in the community is waiting on you to fix the issue is an extra motivator to make quick changes. you should push changes still to your repo: https://github.com/UCLouvain-CBIO/CytoPipeline . the package is still in pre-review phase. once these issues with dependencies are straightened out, we'll move forward and create the repo on git.bioconductor.org and then you would have to push to both.

[phauchamps]

Thanks Lori. I have posted an issue on ggcyto repo. It is being handled by Mike Jiang. I will let you know when I have pushed a working version again. Kind regards, Philippe

[phauchamps]

Hi Lori and Bioc team, The issues related to dependent package maintenance are now closed. Could you please resume the CytoPipeline package submission process, based on the new repo state ? Thanks! Philippe

[bioc-issue-bot]

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]

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

[bioc-issue-bot]

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

[bioc-issue-bot]

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

[PeteHaitch]

Hi @phauchamps,

Just a heads-up that I won't be able to begin my review of CytoPipeline until Friday next week.

Cheers, Pete

[bioc-issue-bot]

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

[bioc-issue-bot]

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

[PeteHaitch]

Hi @phauchamps,

Thank you for submitting CytoPipeline to Bioconductor. Overall, the package is in great shape, nice work!

In my review below I have separated the issues into Required and Recommended points that I would ask you to address before the package can be accepted. Would you please provide line-by-line comments to my initial review so that I know what changes I'm looking for in my re-review.

Cheers, Pete

Required

• [ ] The vignette is missing an echoed library(CytoPipeline) chunk at the beginning. Without that, a user copy-pasting the code would get an error when they run the first chunk of code.
• [ ] Are CytoPipelineGUI and CytoPipelineUtils going to be submitted to Bioconductor? The inclusion in the vignette of code related to CytoPipelineGUI feels a bit funny given it's only available on GitHub (and Bioconductor aims to restrict dependencies to mainstream repositories such as CRAN and BioC).
• [ ] The Bioconductor guidelines recommend not include LazyData: TRUE unless there are exceptions (and if so to please provide them as part of the review).
  • Aside: you currently use LazyData: true; I'm unsure if there is a true/TRUE distinction.
• [ ] BiocCheck::BiocCheck() currently notes NOTE: License 'GPL (>=3) | file LICENSE' unknown; licenses cannot restrict use; would you please confirm that the license does not restrict use. I think this is raising a note because it's not a common license that BiocCheck::BiocCheck() knows how to handle. We may need someone from @Bioconductor/core to comment on its suitability.
• [ ] Please add an Installation section to the vignette.

Recommended

• [ ] In the vignette, perhaps replace .scripts/MakeOMIP021Samples with r system.file("scripts/MakeOMIP021Samples.R", package = "CytoPipeline")? I think the user/reader would need to evaluate this code themselves to get the correct path because if the vignette is build on the Bioconductor build system (BBS) then the path would reflect the BBS rather than the user's machine, but I'm not certain.
• [ ] Why withr::local_tempdir() and not just base::tempdir()?
• [ ] show(popL_PeacoQC) has some extraneous whitespace in what's printed by the show()` method, e.g., there appear to be double the intended whitespace in some of this output.

> show(pipL_PeacoQC)
Pipeline object for flow cytometry experiment:  OMIP021_PeacoQC 
Sample files: 2  sample file(s)
Scale transformations evaluation queue  has no processing step
Flow frames pre-processing evaluation queue  has no processing step

• [ ] Please use file.path() instead of paste0() to construct file paths (see ?file.path for reasoning).
  • e.g., in the vignette, file.path(jsonDir, "OMIP021_flowAI_pipeline.json") instead of paste0(jsonDir, "/")
• [ ] From the vignette, there are a number of warnings when running execute(pipL_PeacoQC, path = workDir) and withr::with_dir(jsonDir, execute(pipL_flowAI, path = workDir)). For a new user, it can be helpful to add some commentary to the vignette about whether this is typical or whether it is because of some features of the example data and so these can be safely ignored in this case.
• [ ] In the vignette, I recommend additional spacing/separation after output of execute(pipL_PeacoQC, path = workDir) and before code chunk starting # execute flowAI pipeline because I nearly scrolled past and missed the second chunk.
• [ ] Captions for figures in vignette are strongly recommended.
• [ ] You might write "themself" instead of using the binary "her/himself" (which is still much better than just "himself", so thank you 🏳️‍🌈).
• [ ] Considering adding a inst/CITATION file.
• [ ] Consider adding a package man page.
• [ ] Great work with the unit tests! covr::report() may help identify any areas for improvement.

[phauchamps]

Hi @PeteHaitch,

Thanks for reviewing my package and for your comments and suggestions!

Herebelow are my line by line comments.

Cheers,

Philippe

Required

• [x] The vignette is missing an echoed library(CytoPipeline) chunk at the beginning. Without that, a user copy-pasting the code would get an error when they run the first chunk of code. => ANSWER: OK, will do!
• [x] Are CytoPipelineGUI and CytoPipelineUtils going to be submitted to Bioconductor? The inclusion in the vignette of code related to CytoPipelineGUI feels a bit funny given it's only available on GitHub (and Bioconductor aims to restrict dependencies to mainstream repositories such as CRAN and BioC). => ANSWER: regarding CytoPipelineGUI, most probably yes, because it contains visualization key functionalities, but the GUI is currently not stable yet is still evolving a lot, so this will be for sometime in the future. Regarding CytoPipelineUtils, this package contains a series of wrappers that are meant to be used within a CytoPipeline run. Previously these were included in CytoPipeline, but this turned out out to create lots of dependencies in the CytoPipeline package, and painful to maintain properly. Now the idea is that CytoPipelineUtils gathers all kind of wrappers, including contributions from users. So I don't think I will rush to have it included into Bioconductor, maybe some time in the future, but I am not sure. Note that the vignette itself does not contain any code from CytoPipelineUtils (even commented out).

• [x] The Bioconductor guidelines recommend not include LazyData: TRUE unless there are exceptions (and if so to please provide them as part of the review). => ANSWER: OK, I will remove it.

  • Aside: you currently use LazyData: true; I'm unsure if there is a true/TRUE distinction. => ANSWER I don't think there is any distinction. I think that LazyData: true was generated automatically by usethis::usedata()
• [x] BiocCheck::BiocCheck() currently notes NOTE: License 'GPL (>=3) | file LICENSE' unknown; licenses cannot restrict use; would you please confirm that the license does not restrict use. I think this is raising a note because it's not a common license that BiocCheck::BiocCheck() knows how to handle. We may need someone from @Bioconductor/core to comment on its suitability. => ANSWER: The license terms are GPL (>=3), the license file does not add any restriction, it is just made explicit with some template of my institution. I think that the reason I kept it in DESCRIPTION that otherwise BiocCheck was still complaining about a LICENSE file being present, but I'll check and possibly try to amend a bit the DESCRIPTION file. See also the following blogpost from my PI, Laurent Gatto.
• [x] Please add an Installation section to the vignette. => ANSWER: ok, will do !

Recommended

• [x] In the vignette, perhaps replace .scripts/MakeOMIP021Samples with r system.file("scripts/MakeOMIP021Samples.R", package = "CytoPipeline")? I think the user/reader would need to evaluate this code themselves to get the correct path because if the vignette is build on the Bioconductor build system (BBS) then the path would reflect the BBS rather than the user's machine, but I'm not certain. => ANSWER: OK, will make that clearer.
• [x] Why withr::local_tempdir() and not just base::tempdir()? => ANSWER: no particular reason, I was just unaware of the base::tempdir() version. I will amend it.
• [x] show(popL_PeacoQC) has some extraneous whitespace in what's printed by the show()` method, e.g., there appear to be double the intended whitespace in some of this output. => ANSWER: you are right, I will check.

> show(pipL_PeacoQC)
Pipeline object for flow cytometry experiment:  OMIP021_PeacoQC 
Sample files: 2  sample file(s)
Scale transformations evaluation queue  has no processing step
Flow frames pre-processing evaluation queue  has no processing step

• [x] Please use file.path() instead of paste0() to construct file paths (see ?file.path for reasoning).

  • e.g., in the vignette, file.path(jsonDir, "OMIP021_flowAI_pipeline.json") instead of paste0(jsonDir, "/") => ANSWER: makes sense, will do!
• [x] From the vignette, there are a number of warnings when running execute(pipL_PeacoQC, path = workDir) and withr::with_dir(jsonDir, execute(pipL_flowAI, path = workDir)). For a new user, it can be helpful to add some commentary to the vignette about whether this is typical or whether it is because of some features of the example data and so these can be safely ignored in this case. => ANSWER: these warnings come from the packages I am using in my illustrating pipeline (flowAI, peacoQC). I will explicitely comment on these warnings in the vignette, in order to avoid confusing users.
• [x] In the vignette, I recommend additional spacing/separation after output of execute(pipL_PeacoQC, path = workDir) and before code chunk starting # execute flowAI pipeline because I nearly scrolled past and missed the second chunk. => ANSWER: ok, will do!
• [x] Captions for figures in vignette are strongly recommended. => ANSWER: ok, will do !
• [x] You might write "themself" instead of using the binary "her/himself" (which is still much better than just "himself", so thank you 🏳️‍🌈). => ANSWER: I did not know that, thanks for mentioning this :-)
• [x] Considering adding a inst/CITATION file. => ANSWER: as there is an article in preparation about CytoPipeline, I will wait for the manuscript to be issued (e.g. as a pre-print), before customizing the CITATION.
• [x] Consider adding a package man page. => ANSWER: ok, will do !
• [x] Great work with the unit tests! covr::report() may help identify any areas for improvement. => ANSWER: thank you! Actually, recently I had to deactivate some of my unit tests, due to a regression bug in ggplot2. I expect to put them back soon, when the bug correction has been released with ggplot2 version 3.4.1. This should increase the code coverage to 80%+ :-)

[PeteHaitch]

Thanks, @phauchamps .

Just a reminder to please push the changes to git@git.bioconductor.org:packages/CytoPipeline so that the build system can re-build the package as part of the review. I'll await that before finalising my review.

Thanks, Pete

[phauchamps]

Hi @PeteHaitch, just to mention that due to other duties for this week, I will start implementing the agreed changes next week, so you should have a new push by end of next week (at the latest).

Cheers,

Philippe

[PeteHaitch]

Hi Philippe,

No problem and thank you for letting me know.

Cheers, Pete

[bioc-issue-bot]

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

[bioc-issue-bot]

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

[phauchamps]

Hi @PeteHaitch,

Just to let you know that I have commited/pushed all my changes to the repo (version 0.99.5).

Cheers,

Philippe

[PeteHaitch]

Thank you for making the requested changes, @phauchamps. I'm happy to accept CytoPipeline into Bioconductor. Congratulations and thank you for your contribution!

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

[phauchamps]

Thank you @PeteHaitch ! :-)

[lshep]

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

https://bioconductor.org/packages/CytoPipeline

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.