Contributions icon indicating copy to clipboard operation
Contributions copied to clipboard

epiregulon

Open TomVuod opened this issue 1 year ago • 44 comments

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

  • Repository: https://github.com/xiaosaiyao/epiregulon

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.

TomVuod avatar Jan 15 '24 08:01 TomVuod

Hi @TomVuod

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: epiregulon
Title: Gene regulatory network inference from single cell epigenomic data
Version: 0.99.0
Date: 2023-11-21
Authors@R: 
    c(person(given = "Xiaosai", family = "Yao", role = c("aut", "cre"), email = "[email protected]", comment = c(ORCID = "0000-0001-9729-0726")),
    person(given = "Tomasz", family = "Włodarczyk", role = c("aut"), email = "[email protected]", comment = c(ORCID = "0000-0003-1554-9699")),
    person(given = "Aaron", family = "Lun", role=c("aut"), email="[email protected]"),
    person(given = "Shang-Yang", family = "Chen", role = "aut", email = "[email protected]"))
Description: Gene regulatory networks model the underlying gene regulation hierarchies that drive gene expression and observed phenotypes. Epiregulon infers TF activity in single cells by constructing a gene regulatory network (regulons). This is achieved through integration of scATAC-seq and scRNA-seq data and incorporation of public bulk TF ChIP-seq data. Links between regulatory elements and their target genes are established by computing correlations between chromatin accessibility and gene expressions.
License: file LICENSE
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.3
Imports:
    BiocFileCache,
    BiocParallel,
    Matrix,
    Rcpp,
    S4Vectors,
    SummarizedExperiment,
    bluster,
    checkmate,
    entropy,
    lifecycle,
    methods,
    scran,
    scuttle,
    stats,
    utils,
    scMultiome,
    GenomeInfoDb,
    GenomicRanges,
    AUCell,
    BSgenome.Hsapiens.UCSC.hg19,
    BSgenome.Hsapiens.UCSC.hg38,
    BSgenome.Mmusculus.UCSC.mm10,
    motifmatchr
Depends: 
    R (>= 4.2.0),
    SingleCellExperiment
Suggests: 
    knitr,
    rmarkdown,
    parallel,
    BiocStyle,
    testthat (>= 3.0.0),
    coin,
    scater
LinkingTo: Rcpp
VignetteBuilder: knitr
URL: https://github.com/xiaosaiyao/epiregulon/
biocViews: GeneExpression, Transcription, ChipOnChip, DifferentialExpression, GeneTarget, Normalization, GraphAndNetwork
Config/testthat/edition: 3
BugReports: https://github.com/xiaosaiyao/epiregulon/issues

bioc-issue-bot avatar Jan 15 '24 08:01 bioc-issue-bot

I had a quick look. I think it would be helpful to readers/reviewers if the vignette referenced the online book https://xiaosaiyao.github.io/epiregulon.book/ . The vignette introduction is too terse and does not explain terms used.

vjcitn avatar Jan 15 '24 11:01 vjcitn

Thank you @vjcitn. We've now added the reference to the book in the vignette

xiaosaiyao avatar Jan 15 '24 17:01 xiaosaiyao

We normally don't allow data to be hosted on an untrusted source like github, dropbox, etc. Can the motif data be submitted to a more stable location like zenodo?

lshep avatar Jan 26 '24 16:01 lshep

Hi @lshep chromVARmotifs is a highly cited and widely used package. https://github.com/GreenleafLab/chromVARmotifs

We prefer to download directly from the source. I hope that's acceptable.

xiaosaiyao avatar Jan 26 '24 17:01 xiaosaiyao

The chromVARmotifs has not been updated in 7 years, has several open issues with no response, and there is no set of action to perform any sort of systematic checking (like R CMD build, R CMD check, etc.) which is why Bioconductor does not allow github only packages to be listed as dependencies and only allows packages on CRAN or Bioconductor to be a dependency. The same is true for data pulled from those sources; there is no clear indication of maintenance or keeping data up-to-date. chromVARmotifs should submit their package to CRAN or Bioconductor; or at least add their data to a public curation site like zenodo, microsoft data lakes, or public S3 bucket.

lshep avatar Jan 29 '24 15:01 lshep

Hello @Ishep, epiregulon in no longer dependent on chromVARmotifs repository. The data sets have been included in scMultiome Bioc package and are retrieved from AnnotationHub (commit 355c2f9). We are now waiting for the release of rhdf5 package v2.47.4 since the bug in the current version causes error when vignette is being built (https://github.com/grimbough/rhdf5/issues/139).

TomVuod avatar Feb 12 '24 10:02 TomVuod

It looks like the builders missed the Sat build. I would expect the new version of rhdf5 to be available later today, after today's builds and propagation, once that happens I'll re-evaluate the package for the next stage of review.

lshep avatar Feb 12 '24 13:02 lshep

You still have a library call to chromVARmotifs in the documentation. Please remove. Since you now call AnnotationHub/ExperimentHub you will likely need to list them in your DESCRIPTION file appropriately in Suggests

lshep avatar Feb 14 '24 14:02 lshep

Can you use an OSI-approved license for this package? https://opensource.org/licenses/

Fiscal sponsorship through NumFocus obligates us to support software packages with open source licensing.

vjcitn avatar Feb 14 '24 17:02 vjcitn

@vjcitn I just consulted with our legal team. We would prefer to stick to our existing license which gives open access to all academic centers. The only restriction we impose is for commercial entities. I hope our license is compliant and will not block our submission to BioC.

xiaosaiyao avatar Feb 14 '24 18:02 xiaosaiyao

Hello @Ishep , the documentation and DESCRIPTION has been corrected.

TomVuod avatar Feb 16 '24 06:02 TomVuod

https://contributions.bioconductor.org/license.html is very clear that restrictive licensing (e.g., academic only) is not possible. The project fiscal sponsorship has required us to avoid restrictive licensing. I think that if you cannot use open licensing we probably shouldn't pursue this submission, really sorry about that.

vjcitn avatar Feb 26 '24 18:02 vjcitn

@vjcitn regretfully we can't change the LICENCE (at least for now) and have to withdraw this submission. FYI @TomVuod

xiaosaiyao avatar Mar 07 '24 05:03 xiaosaiyao

I am sorry about this. I will have further discussions with NumFocus to see if there is a way forward.

vjcitn avatar Mar 07 '24 10:03 vjcitn

@vjcitn we have received the green light to change the license. It is now MIT.

FYI @TomVuod

xiaosaiyao avatar Mar 17 '24 23:03 xiaosaiyao

That's great, we'll start the review process soon.

vjcitn avatar Mar 17 '24 23:03 vjcitn

I've updated my source archive. A few questions to be resolved before review begins.

  • getTFMotifInfo doc says #' motif information is annotated by cisbp from [chromVARmotifs](https://github.com/GreenleafLab/chromVARmotifs) what does "is annotated" mean? Are there real-time queries to chromVARmotifs? Was a snapshot made? If so what kind of provenance information can be provided?

  • Vignette section 4.1 has First, we retrieve the information of TF binding sites collected from Cistrome and ENCODE ChIP-seq, which are hosted on Genomitory. What is Genomitory?

  • I'd say the introduction of the vignette is too terse, "AR" should be spelled out too. Give a brief account of the scientific purpose of the package in the vignette introduction.

  • 4.4 has

"A long format dataframe, representing the inferred regulons, is then generated. The dataframe consists of three columns:

tf (transcription factor) target gene peak to gene correlation between tf and target gene"

but the code output does not produce such a data frame?

regulon <- getRegulon(p2g = p2g, overlap = overlap, aggregate = FALSE)
regulon
#> DataFrame with 746875 rows and 10 columns
#>          idxATAC         chr     start       end    idxRNA      target
#>        <integer> <character> <integer> <integer> <integer> <character>
#> 1             37        chr1   1020509   1021009        22  AL645608.4
#> 2             37        chr1   1020509   1021009        22  AL645608.4
#> 3             37        chr1   1020509   1021009        22  AL645608.4
  • the vignette concludes with head(score.combine) but the output in chrome browser looks like

image

vjcitn avatar Mar 18 '24 11:03 vjcitn

@vjcitn Thank you for reviewing the vignette. I apologize for not updating some of the text earlier. The vignette should be cleaned up. I provided further details on the package in the introduction. Let me know should you require more changes.

xiaosaiyao avatar Mar 19 '24 00:03 xiaosaiyao

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 Mar 21 '24 12:03 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: "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.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): epiregulon_0.99.6.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/epiregulon to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Mar 21 '24 13:03 bioc-issue-bot

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 Mar 21 '24 16:03 bioc-issue-bot

@lshep do I have the permission to push to upstream? or only @TomVuod has?

xiaosaiyao avatar Mar 21 '24 18:03 xiaosaiyao

The BiocCredentials account is now wrong and corrupt for your two account. We assume the submitter is the maintainer and match the submitter github userid with the maintainer email in the DESCRIPTION. So now [email protected] is associated with TomVuod and not xiaosaiyao . I will delete and reset the account and send activation instructions. Do you both need push access?

lshep avatar Mar 21 '24 18:03 lshep

@lshep It will be great to have both. Thank you!

xiaosaiyao avatar Mar 21 '24 18:03 xiaosaiyao

Please both look for an email entitled BiocCredentials account

lshep avatar Mar 21 '24 18:03 lshep

It works now. Thank you! @lshep

xiaosaiyao avatar Mar 21 '24 18:03 xiaosaiyao

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

bioc-issue-bot avatar Mar 21 '24 18:03 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: "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.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): epiregulon_0.99.7.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/epiregulon to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot avatar Mar 21 '24 19:03 bioc-issue-bot

AdditionalPackage: https://github.com/xiaosaiyao/epiregulon.extra

TomVuod avatar Mar 21 '24 19:03 TomVuod