(inactive) TSSr-submission

Linlab-slu commented 3 years ago

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

Repository: https://github.com/Linlab-slu/TSSr

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 https://bioconductor.org/developers/package-submission/ instructions. My package is consistent with the Bioconductor https://contributions.bioconductor.org/.
[x] I understand Bioconductor https://bioconductor.org/developers/package-submission/#naming 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 https://support.bioconductor.org for issues that users may have, subscribing to the https://stat.ethz.ch/mailman/listinfo/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 https://bioconductor.org/about/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 http://bioconductor.org/developers/how-to/git/ (optionally http://bioconductor.org/developers/how-to/git/sync-existing-repositories/).

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 https://bioconductor.org/ to sign up.

bioc-issue-bot commented 3 years ago

Hi @Linlab-slu

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:

Type: Package
Package: TSSr
Version: 0.99.0
Date: 2021-08-02
Title: TSS sequencing data analysis
Description: TSSr package provides a comprehensive workflow on TSS data starts from 
    identification of accurate TSS locations, clustering TSSs within small genomic 
    regions corresponding to core promoters, and transcriptional activity quantifications, 
    as well as specialized downstream analyses including core promoter shape, cluster 
    annotation, gene differential expression, core promoter shift. TSSr can take 
    multiple formats of files as input, such as Binary Sequence Alignment Mao (BAM) 
    files (single-ended or paired-ended), Browser Extension Data (bed) files, BigWig 
    files, ctss files or tss tables. TSSr also generates various types of TSS or core 
    promoter track files which can be visualized in the UCSC Genome Browser or Integrative 
    Genomics Viewer (IGV). TSSr also exports downstream analyses result tables and plots. 
    Multiple cores are supported on Linux or Mac platforms.
Authors@R: c(person("Zhaolian","Lu",email="luzhaolian@gmail.com",role=c("aut","com"),
          comment = c(ORCID = "0000-0001-5002-7007")),
   person("Keenan","Berry",email="keenan.berry@slu.edu",role=c("aut","com")),
   person("Zhenbin","Hu",email="zhenbin.hu@slu.edu",role=c("aut","ctb")),
   person("Yu","Zhan",email="yu.zhan@slu.edu",role=c("aut","ctb")),
   person("Tae-Hyuk (Ted)","Ahn",email="ted.ahn@slu.edu",role=c("aut","cph")),
   person("Zhenguo","Lin",email="zhenguo.lin@slu.edu",role=c("aut","cre","cph"),
          comment = c(ORCID = "0000-0002-8400-9138")))
License: file LICENSE
Encoding: UTF-8
LazyData: FALSE
Depends: R (>= 4.0)
Imports: stringr, rtracklayer,ggplot2, GenomicFeatures, Gviz, DESeq2,
    BSgenome.Scerevisiae.UCSC.sacCer3, calibrate,ggfortify, IRanges,
    GenomicRanges, Rsamtools, GenomicAlignments, GenomeInfoDb,dplyr,
    BiocGenerics, data.table, methods, grDevices, graphics, stats, utils,parallel
Suggests: knitr, rmarkdown, pkgdown
biocViews: Software, Transcription, Coverage, GeneExpression,
    GeneRegulation, PeakDetection, DataImport, DataRepresentation,
    Transcriptomics, Sequencing, Annotation, GenomeBrowsers,
    Normalization, Preprocessing, Visualization,DifferentialExpression, 
    Alignment,Clustering
URL: https://github.com/Linlab-slu/TSSr
BugReports: https://github.com/Linlab-slu/TSSr/issues
VignetteBuilder: knitr
RoxygenNote: 7.1.1
NeedsCompilation: no
git_url: https://github.com/Linlab-slu/TSSr
git_branch: RELEASE_1
git_last_commit: a5016e4
git_last_commit_date: 2021-08-02
Date/Publication: 2021-08-02
Packaged: 2021-08-02; biocbuild

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

hpages commented 3 years ago

Hi @Linlab-slu ,

Please address the errors and warnings reported by the SPB (Single Package Builder). Thanks!

Best, H.

bioc-issue-bot commented 3 years ago

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

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

bioc-issue-bot commented 3 years ago

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

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

bioc-issue-bot commented 3 years ago

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

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

hpages commented 3 years ago

Hi @Linlab-slu ,

Thanks for fixing the errors. BiocCheck("TSSr_0.99.3.tar.gz") still reports many NOTEs: 14 in total, which is a lot! Many of them deserve some attention and should be easy to address. For example:

    * NOTE: 'sessionInfo' not found in vignette(s)
    ...
    * NOTE: Avoid sapply(); use vapply()
    ...
    * NOTE: Avoid 1:...; use seq_len() or seq_along()
    ...
    * NOTE: Avoid redundant 'stop' and 'warn*' in signal conditions
    ...
    * NOTE: Consider adding runnable examples to the following man
      pages which document exported objects:
      callEnhancer.Rd, clusterTSS.Rd, getTSS.Rd, shapeCluster.Rd,
      shiftPromoter.Rd

Please reduce the number of NOTEs to <= 8.

Thanks, H.

hpages commented 3 years ago

Hi @Linlab-slu ,

Were you able to take a look at this?

Thanks, H.

Linlab-slu commented 3 years ago

@hpages

Sorry for the late updates. Yes, I would definitely reduce some notes and repush as soon as possible.

Thanks a lot!

bioc-issue-bot commented 3 years ago

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

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

hpages commented 3 years ago

You've only addressed 1 NOTE out of the 14 NOTEs. Is this the best you can do? I've pointed you to other NOTEs that deserve your attention and should be easy to address. Thanks!

H.

bioc-issue-bot commented 3 years ago

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

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

bioc-issue-bot commented 3 years ago

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

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

hpages commented 3 years ago

Thanks for the improvements @Linlab-slu.

A few more things:

Why are the examples in the man pages for callEnhancer(), getTSS(), and shapeCluster() commented out? Per our guidelines, all user facing functions need to have at least one working example.
Why does the \value section in the man page for callEnhancer() say that the function returns a "Large List of elements - one element for each sample" when this function seems to return a TSSr object? (Note that TSSr is an S4 class and TSSr object have slots, while an ordinary list has list elements).
The TSSr object returned by callEnhancer() contains data.table objects (e.g. in the PromoterShift slot). Unfortunately, while it's fine to use data.table objects internally in your package, we discourage exposing these objects to the end user. This is because they don't obey the "standard data.frame API" e.g. DT[2] extracts the 2nd row when it should extract the 2nd column. It wouldn't be a big problem if data.table was not extending data.frame, but it does. Concretely this means that the huge amount of code around (in Bioconductor and CRAN packages) that was written in a generic fashion to work on objects for which is.data.frame() is TRUE is most likely going to choke on a data.table object. This is an unfortunate situation and I wish the data.table developers had stuck to the standard data.frame contract here. So please make sure to always present data.frame or DataFrame objects instead of data.table objects to the end user.

Thanks, H.

zhaolianlu commented 3 years ago

Hi, @hpages

Thanks for your comments! Here are my explanations regarding your concerns:

These functions are the speed limit steps in CMD check, so I excluded them out for checking.
No. callEnhancer() function returns list of data tables back into the object in the . There is nothing to output or print externally when running callEnhancer() functions.
That is a good point. We have many slots containing data.table objects internally. I checked the whole pipeline, and believed we don't expose data.table objects to the end user, except when the users type object name to see what's in it. That's should be fine from my sense. If the user want to access the results from some functions like callEnhancer(), we provided a list of functions, such as exportEnhancerTable(), to export the whole table into a file.

Thanks a lot! Please let me know if you still have any concerns. ZL

hpages commented 2 years ago

Hi @zhaolianlu ,

Sorry for the delay.

Item 1:

These functions are the speed limit steps in CMD check, so I excluded them out for checking.

But it takes only 0.007 s to run the exportEnhancerTable example on my laptop:

system.time(res2 <- exportEnhancerTable(exampleTSSr))
# Exporting enhancer table...
#    user  system elapsed 
#   0.004   0.000   0.007

and only 2.18 s to run the callEnhancer example:

system.time(res <- callEnhancer(exampleTSSr,flanking = 400,dis2gene=2000))
# Calculating enhancers...
#    user  system elapsed 
#   2.171   0.048   2.180 
# Warning messages:
# 1: In `[.data.table`(cs, , `:=`(gene, NULL)) :
#   Column 'gene' does not exist to remove
# 2: In `[.data.table`(cs, , `:=`(inCoding, NULL)) :
#   Column 'inCoding' does not exist to remove
# 3: In `[.data.table`(cs, , `:=`(gene, NULL)) :
#   Column 'gene' does not exist to remove
# 4: In `[.data.table`(cs, , `:=`(inCoding, NULL)) :
#   Column 'inCoding' does not exist to remove

Anyways, disabling examples is not the way to go as it almost guarantees that the code will rot at some point. We can already see some warnings when running the above code which suggests that the code rot has already started, maybe? Per our guidelines, all user-facing functions must be documented and have running examples. Please make sure to comply and also make sure to get rid of these warnings as they can only confuse the end user.

Item 2:

Here is how the value returned by callEnhancer() is described in the man page:

Large List of elements - one element for each sample

However, what I get is a TSSr object, not a list:

> class(res)
[1] "TSSr"
attr(,"package")
[1] "TSSr"

So your description of the returned value is inaccurate and way too vague. What's exactly in this object, how to access its content, and, more generally, what to do with it, should be precisely documented.

Item 3:

As long as the end user never has to manipulate directly those data.table objects we should be fine.

One more thing:

Item 4:

It looks like you've taken the approach of making all your functions S4 generics, in a systematic way. There's no need for this and it's not considered good practice (it adds an unnecessary layer of complexity and makes it harder to debug the functions). Please use regular functions instead.

Thanks, H.

hpages commented 2 years ago

Dear @zhaolianlu, do you intend to follow up with this submission? Thanks. H.

lshep commented 2 years ago

@zhaolianlu When you are ready with changes and to continue with the review process please request that this issue be reopened.

bioc-issue-bot commented 2 years ago

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.

Bioconductor / Contributions

(inactive) TSSr-submission #2222