Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

scMultiome #2901

Closed xiaosaiyao closed 1 year ago

xiaosaiyao commented 1 year ago

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

Confirm the following by editing each check box to '[x]'

I am familiar with the essential aspects of Bioconductor software management, including:

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 1 year ago

Hi @xiaosaiyao

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: scMultiome
Title: Collection of Public Single-Cell Multiome (scATAC + scRNAseq) Datasets
Version: 0.99.5
Authors@R: c(
    person(given = "Xiaosai", family ="Yao", email = "yao.xiaosai@gene.com", role = c("cre","aut"),  comment = c(ORCID = "0000-0001-9729-0726")),
    person(given = "Aleksander", family ="Chlebowski", email = "aleksander.chlebowski@contractors.roche.com", role = "aut"),
    person(given = "Aaron", family ="Lun", email = "lun.aaron@gene.com", role = "aut"),
    person(given = "Shiqi", family ="Xie", email = "xie.shiqi@gene.com", role = "ctb"),
    person(given = "Tomasz", family = "Wlodarczyk", email = "tomasz.wlodarczyk@contractors", role = "aut"),
    person(given = "Natalie", family = "Fox", email = "natalie.fox@roche.com", role = "aut")
  )
Description: Single cell multiome data, containing chromatin accessibility
    (scATAC-seq) and gene expression (scRNA-seq) information analyzed with
    the ArchR package and presented as MultiAssayExperiment objects.
License: file LICENSE
Depends: 
    R (>= 4.3.0),
    AnnotationHub,
    ExperimentHub,
    MultiAssayExperiment,
    SingleCellExperiment,
    SummarizedExperiment
Imports: 
    AzureStor,
    DelayedArray,
    GenomicRanges,
    HDF5Array,
    S4Vectors,
    checkmate,
    methods,
    rhdf5
Suggests: 
    BiocGenerics,
    IRanges,
    Matrix,
    knitr,
    rmarkdown,
    rstudioapi,
    testthat (>= 3.0.0),
    devtools,
    BiocStyle,
    ExperimentHubData
VignetteBuilder: 
    knitr
Config/testthat/edition: 3
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.3
biocViews: PackageTypeData, ExperimentHub, SingleCellData, ExpressionData,
    SequencingData, Homo_sapiens_Data, CellCulture, Tissue, GEO
bioc-issue-bot commented 1 year 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 1 year 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/scMultiome to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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/scMultiome to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

xiaosaiyao commented 1 year ago

I have trouble verifying whether I have subscribed to the mailing list with yao.xiaosai@gene.com

I thought I did but I cannot log in or retrieve password reminder

lshep commented 1 year ago

I do not see you registered. Please check spam folders. there is normally a confirmation email that gets sent and sometimes it is marked as spam

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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/scMultiome to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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/scMultiome to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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/scMultiome to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

xiaosaiyao commented 1 year ago

I still have a warning about unevaluated chunks. These chunks are for demonstrating how to upload new data and cannot be evaluated. Can we still proceed with the review?

lgatto commented 1 year ago

I spotted this new submission, which looks really interesting and I'll look more into it. I just also wanted to point to SingleCellMultiModal, which seems to address similar needs. I would suggest @xiaosaiyao to cite SingleCellMultiModal in their vignette intro and highlight any commonalities and differences. Once scMultimodal makes it into Bioc, I would suggest to send a PR to SingleCellMultiModal to update that vignette accordingly (and I'm happy to be reminded to do so if I forget).

xiaosaiyao commented 1 year ago

Thank you for the suggestion @lgatto. I will cite SingleCellMultiModal in my vignette

hpages commented 1 year ago

Hi @xiaosaiyao,

Thanks for submitting scMultiome. Can you please clarify what license is being used for the package? Based on the LICENSE file it looks like it's one of the Creative Commons licenses, but the file is not very clear about which one exactly. Also the file has many formatting issues, suggesting that maybe it's not the original version. It would be much better if the License field could give the exact name of the license. See the list of all Creative Commons licenses here.

Also please be aware that using a Creative Commons license for software is unusual and not recommended. From the wikipedia article:

While software is also governed by copyright law and CC licenses are applicable, the CC recommends against using it in software specifically due to backward-compatibility limitations with existing commonly used software licenses.[17][18] Instead, developers may resort to use more software-friendly free and open-source software (FOSS) software licenses.

I'm going to take a closer look at the package now and will let you know if I have more feedback.

H.

xiaosaiyao commented 1 year ago

Thank you @hpages. The reason for having the Creative Commons license is because one of the datasets in this package came from ChIP-Atlas whose data follows a Creative Commons license and after consulting with our legal team, we decided to follow their license. I will update the license to the exact name.

hpages commented 1 year ago

Hi @xiaosaiyao,

I noticed that the scMultiome relies a lot on the ArchR package to demonstrate some important operations, but unfortunately ArchR is not a Bioconductor or CRAN package. This leads to the following situation: ArchR is used in the vignette and in various man pages but none of the code that demonstrates its use is actually evaluated, with the consequence that this code cannot be validated via the standard R CMD build and R CMD check commands.

Note that we insist that new contributions to Bioconductor interact with the existing Bioconductor ecosystem, which means that they should not rely on R packages that are not part of it. Disabling all the code in the vignette and examples that use ArchR is not considered good practice, and also doesn't address the real problem that ArchR is not part of the ecosystem in the first place. Have you considered discussing the submission of the ArchR package to Bioconductor with its authors/maintainers?

Other than that, I spotted 2 minor issues:

Thanks

xiaosaiyao commented 1 year ago

Thank you so much for your comments @hpages. I will correct the minor issues.

Indeed, the integration between scMultiome and ArchR was intended to be seamless but was made challenging by the fact that ArchR is part of the BioC ecosystem. In fact, I am actively collaborating with @rcorces, the maintainer of ArchR. I have spoken with @rcorces on this and the issue is that many of ArchR's dependencies are not in BioC/CRAN. @rcorces can chime in more. In situations like this, what do you recommend @hpages ?

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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, 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/scMultiome to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

hpages commented 1 year ago

... but was made challenging by the fact that ArchR is part of the BioC ecosystem.

You mean "by the fact that ArchR is NOT part of the BioC ecosystem".

In situations like this, what do you recommend?

I would recommend focusing on getting ArchR's dependencies on CRAN or Bioconductor. Then on getting ArchR itself on Bioconductor. Then finally on getting scMultiome in Bioconductor. This is the logical order. I'm afraid that trying to start with scMultiome is like trying to put the cart before the horse, unfortunately.

Best, H.

xiaosaiyao commented 1 year ago

Thank you for your suggestions, @hpages

Combining input from @rcorces, here is my reply:

  1. ArchR has 3 dependencies not currently in BioC/CRAN and this includes Monocle3. At this point, it's really beyond us to convince the other folks to get their packages into CRAN/BioC

  2. I still believe scMultiome should be in BioC because it is an ExperimentHub data package and contains standalone functions independent from ArchR

  3. We have 2 sections called ArchR Multiome Workflow and Conversion to MAE in the vignette. These sections may have created some confusion and misled the reader into thinking that scMultiome requires these 2 steps to function. This is not the case since scMultiome contains multimodal datasets with raw counts that are compatible with other downstream packages. I suggest to remove these 2 sections from the vignette. Our plan is to release a separate package to address ArchR-MAE integration.

I have already incorporated the other changes suggested by you and @lgatto

  1. Update the introduction of the vignette to address the similarities and differences between scMultiome and singleCellMultiModal
  2. Update the introduction of the vignette to properly cite the MultiAssayExperiment package
  3. Use file.path to construct paths rather than paste0

I hope the changes are acceptable

hpages commented 1 year ago

ArchR has 3 dependencies not currently in BioC/CRAN.

It's less than that. Strictly speaking all ArchR dependencies, i.e. all the packages listed in its Depends field, are already in BioC/CRAN. Now it seems that ArchR also relies on some additional packages that can be installed with ArchR::installExtraPackages(), and only one of them, presto, is not in BioC/CRAN at the moment. It's worth noting that presto's README.md file says that the package can be installed with install.packages('presto'), even though that's not actually the case, but might be an indication that the authors intend to have the package on CRAN soon.

Note that for some reason ArchR::installExtraPackages() installs harmony from GitHub even though the package is available on CRAN.

As for monocle3, I don't see it as a dependency, direct or indirect. But it's still worth noting that the DESCRIPTION file in the package has a biocViews field, which suggests that the authors intend to submit the package to Bioconductor soon.

Anyways, the good news is that the situation is not as bad as you might think. In the end, only presto is not on BioC/CRAN yet, but it's not inconceivable that this could change soon :wink:

I suggest to remove these 2 sections from the vignette. Our plan is to release a separate package to address ArchR-MAE integration.

Sounds like a reasonable thing to do.

Thanks for the latest changes. I'll take another look ASAP.

H.

xiaosaiyao commented 1 year ago

Hi @hpages, ArchR does depend on monocle3 in its trajectory analysis. ArchR does not state Monocle3 as a dependency in the DESCRIPTION file but it installs Monocle3 through the .requirePackage function.

I will proceed to remove the two sections from the vignette

xiaosaiyao commented 1 year ago

Two further changes:

  1. Updated the license information in the DESCRIPTION file
  2. Removed data preprocessing section in the vignette since this is not a functionality supported by the scMultiome package. The preprocessing information of every dataset can be found in the metadata as pseudocode or description
bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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/scMultiome to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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/scMultiome to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

rcorces commented 1 year ago

@xiaosaiyao @hpages - just to clarify about ArchR's dependencies.

Some of the things that are being discussed had previously been changed on the dev branch of ArchR. In particular, the DESCRIPTION file was inaccurate in previous versions and I updated it a few months ago but those updates are still sitting on dev (https://github.com/GreenleafLab/ArchR/blob/dev/DESCRIPTION). In particular, monocle3 is now Suggested by ArchR.

xiaosaiyao commented 1 year ago

@hpages Could you please comment on whether the package is good to go with all the latest changes? Thank you!

vjcitn commented 1 year ago

he's away for a few days. he'll be back next week

xiaosaiyao commented 1 year ago

Just checking in on this submission...

hpages commented 1 year ago

Hi @xiaosaiyao ,

Sorry for the delay. Package looks good and is ready for inclusion to Bioconductor. Please delete the HTML vignette located in scMultiome/vignettes/. The HTML vignette will be automatically generated by R CMD build and placed in the resulting source tarball, so it should not be precomputed and placed in the package git repo.

Thanks, H.

bioc-issue-bot commented 1 year ago

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

bioc-issue-bot commented 1 year 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/scMultiome to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 1 year 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.

xiaosaiyao commented 1 year ago

Sorry for the late reply @hpages as I was away for a conference. I have removed the html file and I saw that you have accepted the package. Thank you very much for your invaluable feedback!

lshep commented 1 year ago

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

https://bioconductor.org/packages/scMultiome

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.