Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

(inactive) SignacSlim #2592

Closed Honchkrow closed 2 years ago

Honchkrow commented 2 years 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 2 years ago

Hi @Honchkrow

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: SignacSlim
Type: Package
Title: The slim version of signac.
Version: 0.99.0
Authors@R: c(
  person(given = 'Wei', family = 'Zhang', email = 'w-zhang16@tsinghua.org.cn', role = c('aut', 'cre'))
  )
Maintainer: Wei Zhang <w-zhang16@tsinghua.org.cn>
Description: The slim version of signac.
    The package contains the basic classes and methods from SeuratObject, 
    Seurat and Signac. This package aims to provide upstream function  
    for developing the scATAC-seq algorithm.
Depends: R (>= 4.1.0), methods, Rcpp, RcppEigen
License: GPL-3 | file LICENSE
Encoding: UTF-8
LinkingTo: Rcpp, RcppEigen
Imports: Rcpp, RcppEigen, GenomeInfoDb, GenomicRanges, IRanges, Matrix,
        Rsamtools, fastmatch, future, future.apply, pbapply, tidyr,
        BiocGenerics, SeuratObject, biovizBase, dplyr, hdf5r, stringi,
        stats, tools, S4Vectors, data.table, methods, ggplot2
Suggests: knitr, testthat
RoxygenNote: 7.1.2
SystemRequirements: C++11
Collate: 'FeatureMatrix.R' 'NucleosomeSignal.R' 'RcppExports.R'
        'generics.R' 'fragments.R' 'genomeinfodb-methods.R'
        'granges-methods.R' 'iranges-methods.R' 'objects.R'
        'reexports.R' 'region-enrichment.R' 'utilities.R'
        'visualization.R'
biocViews: Software, Sequencing, ATACSeq, WholeGenome, SingleCell
VignetteBuilder: knitr
NeedsCompilation: yes
Packaged: 2022-03-26 13:07:53 UTC; zhang
Author: Wei Zhang [aut, cre]
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: "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/SignacSlim to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

vjcitn commented 2 years ago
Authors@R: c(
  person(given = 'Wei', family = 'Zhang', email = 'w-zhang16@tsinghua.org.cn', role = c('aut', 'cre'))
  )
Maintainer: Wei Zhang <w-zhang16@tsinghua.org.cn>

The Maintainer data will be computed by R CMD build. Omit from DESCRIPTION

bioc-issue-bot commented 2 years ago

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

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/SignacSlim 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: 32a88875e7fe9d6b55cd93f8bd42e4a1f50d492a

lshep commented 2 years ago

We are having trouble with our windows builder that prevented the build from posting. You can check your results at this location: http://staging.bioconductor.org:8000/jobs/1664/

bioc-issue-bot commented 2 years ago

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

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

PeteHaitch commented 2 years ago

Hi @Honchkrow,

I've got concerns with this package being hosted by Bioconductor.

Firstly, and most importantly: are you coordinating with the Seurat and signac authors (@andrewwbutler, @mojaveazure, @timoast and others) on SignacSlim?

From what I understand, you've taken parts of the signac, Seurat/SeuratObject packages, but you've not properly acknowledged the original authors' work (e.g., in the DESCRIPTION). Furthermore, depending on the license, while you may not require their explicit consent to make such a package (I am not a lawyer), I think it would be polite to discuss and co-ordinate with the original authors any package that relies so heavily on their hard work. E.g., how do you plan to keep this package in-sync with upstream changes to signac and Seurat/SeuratObject? You've also opted for a different license to the original package, which while not necessarily forbidden (I'm not a lawyer) does seem odd.

Cheers, Pete

Honchkrow commented 2 years ago

Hi @PeteHaitch ,

Thans for pointting these important issues.

Firt of all, I apologize for using the wrong license. Signac and Seurat used the MIT License which mean I can use, copy, modify and publish under MIT License restriction. I will modify this fatal error immediately. The origin author will be acknowledged in DESCRIPTION and the copy right holder (Tim Stuart) will be listed in LICENSE.md.

Second, I will contact with the original authors to discuss the distribution of this package. If they do not want this package be distributed, I will contact you to close this issue.

Thank for your help again!

Honchkrow

timoast commented 2 years ago

Thanks @PeteHaitch for flagging this!

Hi @Honchkrow

I appreciate your interest in the Signac package and your efforts in making a contribution here, but I would prefer that this package is not hosted on Bioconductor. Making this package broadly available will likely lead to issues for users loading both Signac and SignacSlim due to having classes and functions defined in two places. It's also unclear how SignacSlim will be kept in sync with changes to Signac. I think it may also cause some confusion for users as to where they should ask for support when encountering bugs in the package.

I do agree that the dependency footprint of Signac can be reduced. By far the heaviest dependency is Seurat, and I've now changed Seurat to be a suggested package, along with a couple of other packages that were not used extensively. I would support any efforts to use other package dependencies conditionally and move them to suggested packages if you'd like to contribute a PR to Signac.

Honchkrow commented 2 years ago

Hi @timoast,

Thanks for your rapid reply.

The same class name and incomplete method in SignacSlim do cause troubles when loading Signac. Sorry for not take into account.

I will clsoe this issue. If there any things I should do, for example, delete the package in bioconductor (git@git.bioconductor.org:packages/SignacSlim) or other things, please let me know.

Thanks you again, @PeteHaitch .

Honchkrow

PeteHaitch commented 2 years ago

Thank you for considering everyone's feedback, @Honchkrow. I'm sure there are more opportunities to help improve scATAC-seq analysis in Bioconductor and interoperability with signac.

Regarding the package and issue: I don't think there's anything more you need to do.