FelixErnst commented 5 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/FelixErnst/Structstrings

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

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 help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

bioc-issue-bot commented 5 years ago

Hi @FelixErnst

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: Structstrings
Title: Structstrings: implementation of the dot bracket annotations with Biostrings
Version: 0.99.0
Date: 2018-02-17
Authors@R: person("Felix G.M.", 
        "Ernst", 
        email = "felix.g.m.ernst@outlook.com", 
        role = c("aut", "cre"))
Description: The Structstrings package implements the widely used to bracket 
    annotation for storing base pairing information in structured RNA.
    Structstrings uses the infrastructure provided by the Biostrings package and 
    derives the DotBracketString and related classes from the BString class. 
    From these base pair table can be produced for in depth analysis. In 
    addition, the loop indices of the base pairs can be retrieved.
    For better efficiency information conversion is implemented in C, inspired 
    to a large extend by the ViennaRNA package.
License: Artistic-2.0
Encoding: UTF-8
biocViews:
    DataImport, DataRepresentation, Infrastructure, Sequencing, Software
Depends:
    R (>= 3.6),
    S4Vectors,
    IRanges,
    Biostrings
LinkingTo:
    IRanges,
    S4Vectors
Imports:
    methods,
    assertive,
    BiocGenerics,
    XVector,
    stringr,
    stringi
Suggests: 
    testthat,
    knitr,
    rmarkdown,
    BiocStyle
VignetteBuilder: knitr
RoxygenNote: 6.1.1
Collate: 
    'Structstrings.R'
    'AllGenerics.R'
    'Structstrings-DotBracket-io.R'
    'Structstrings-DotBracketDataFrame.R'
    'Structstrings-DotBracketString.R'
    'Structstrings-DotBracketStringSet.R'
    'Structstrings-DotBracketStringSetList.R'
    'Structstrings-LoopIndexList.R'
    'Structstrings-StructuredXStringSet.R'
    'Structstrings-alphabet.R'
    'Structstrings-conversion.R'
    'zzz.R'
NeedsCompilation: yes

bioc-issue-bot commented 5 years ago

A reviewer has been assigned to your package. Learn what to expect during the review process.

IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your repository will NOT trigger a new build.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

6f91d01 Update DESCRIPTION

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

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

76e191b fixed letter normalization and LoopIndexList valid...

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

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

2b40032 fixed email typo in DESCRIPTION

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

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

ff62011 fixed man page typo and data man pages

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

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

2aefba0 fixed DESCRIPTION typos

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

FelixErnst commented 5 years ago

Hi @Kayla-Morrell,

just wanted to check in and ask, if you already started with the review. I have a few optimizations in a branch and would merge them to master, if you haven't started, yet. These are just minor changes, which do not affect the overall behavior or code structure. Thanks for any update.

Felix

Kayla-Morrell commented 5 years ago

Hi @FelixErnst,

I have just started the review and have only gotten as far as the documentation of the package. If you wanted to merge the changes that would be fine with me.

Best, Kayla

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

e8aad0a code indentation simplified and removed incosisten... 9fc2ac6 Update Structstrings-DotBracketDataFrame.R b983efd refactored the dotbracket accessor and added a set... 8907581 creation of LoopIndexList object now done in C f3ec99b bugfix for convertAnnotation - names are used prop... 11ab3c0 restructure construction of LoopIndexList in C 9630f9d Update DESCRIPTION 2b82378 Merge pull request #1 from FelixErnst/dev updates...

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

FelixErnst commented 5 years ago

hmm, on Linux it took a bit longer to build. I hope that is not a problem.

Kayla-Morrell commented 5 years ago

It's just a few seconds over 5 minutes so it shouldn't matter too much. I'll continue my review with these changes.

FelixErnst commented 5 years ago

Hi @Kayla-Morrell any news on the review? Thanks for any update.

Kayla-Morrell commented 5 years ago

Hi @FelixErnst,

Thank you for submitting to Bioconductor and thank you for your patience with the review process. Please see the initial review of your package below.

General Package Development

[x] SUGGESTION: When running R CMD BiocCheck, consider adding these automatically suggested biocView: Alignment, SequenceMatching.
[x] SUGGESTION: Consider adding runnable examples to the man page 'Structstrings-internals.Rd' since exported objects are documented.
[x] SUGGESTION: For style reasons, consider shorter lines of > 80 characters. There are 13 lines that are longer than 80 characters (this includes comments and roxygen documentation). R CMD BiocCheck provides the first 6 lines:
- R/Structstrings-conversion.R, line 15
- R/Structstrings-conversion.R, line 254
- R/Structstrings-DotBracket-io.R, line 28
- R/Structstrings-DotBracketDataFrame.R, line 92
- R/Structstrings-DotBracketDataFrame.R, line 106
- R/Structstrings-DotBracketDataFrame.R, line 226
[x] SUGGESTION: For style reasons, consider multiple of 4 spaces (not tabs) for line indents. There are 671 lines that are not. R CMD BiocCheck provides the first 6 lines:
- R/AllGenerics.R, line 7
- R/AllGenerics.R, line 11
- R/AllGenerics.R, line 16
- R/AllGenerics.R, line 17
- R/AllGenerics.R, line 21
- R/AllGenerics.R, line 25

vignette

[x] REQUIRED: The first sentence of the 'Introduction' there seems to a grammatical error. I believe you meant for it to read '...used dot bracket...'.
[x] REQUIRED: There seems to be a missing reference for 'Biostrings'. The link in the second paragraph of the 'Introduction' links back to your vignette.
[x] REQUIRED: Create an 'Installation' section that shows the user how to download and load the package from Bioconductor. For example...

if(!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("Structstrings")

Then move the library() command up to this section and start section 2 with the text about DotBracketString().

[x] REQUIRED: Finish your vignette with a SessionInfo() section.

man pages

DotBracketString

[x] REQUIRED: For the description, there seems to be an extra \ when talking about what the alphabet consists of.

DotBracketStringSet-io

[x] REQUIRED: For the description, the first paragraph talking about the '<>' conflict needs to end in a period not a comma.
[ ] REQUIRED: The last sentence of the description needs to be reworded.
[x] CLARIFICATION: Take another look at the links you use to Biostrings:XStringSet-io. It is my understanding that in order for these links to work in the way you have them it should be \code{\link[package:function]{name}} but XStringSet-io is not a function so I don't think the link is rendering correctly. If I am wrong, please feel free to correct me.

Structstrings-data

[x] REQUIRED: Data man pages must include source information and structure information.

Structstrings

[x] SUGGESTION: This is a good use of a package man page that contains an overview of the package. You may also think of adding links to the main functions.

R code

The General package development section have some suggestions for improvements that can be made to the R code.

[x] REQUIRED: Overall, avoid the use of direct sloct access with @, accessor methods should be created and utilized instead.
[x] REQUIRED: Shouldn't use unexported functions (via :::), this was used extensively with setValidity2 from S4Vectors.

Comment back here with updates that have been made and when the package is ready for a re-review.

Best, Kayla

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

e2250e9 changes for review 256d289 additional update for review 5e86676 man pages updated 0157bbd Merge pull request #2 from FelixErnst/review Revi...

FelixErnst commented 5 years ago

I worked through the review

General Package Development

I followed you advice, except for the indentation and some of the line with width > 80L incase they could not be shortened.

vignette

all done

man pages

all done. The \code{\link[package:function]{name}} works as intended, since the actual name of the man page must be used as function. An alias like readDNAStringSet() cannot be used (It could be used with \link{}, but I like the more explicit version).

R code

slot access:

@structure: replaced by accessor calls

The other calls are purely done for optimization purposes.

@listData: used during construction of DotBracketDataFrame to avoid deconstructing and reconstructing the object.
@unlistData: I could do it via unlisting and relisting. However, this adds additional overhead, which is not necessary in my opinion, since I just make small adjustments to the actual data. (edit: since the class slots are defined in IRanges and no accessors are made available there, I like to avoid adding the accessors since the slots are accessed only internally. Basically it is the same usage as seen in the IRanges package)

I hope the few calls are ok for you.

unexported functions

all calls removed except XStringSet, which is needed for the constructor of DotBracketStringSet

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

FelixErnst commented 5 years ago

@Kayla-Morrell the error is due changes in IRanges not propagating to the SPB. I bump the Structstrings version, when the new IRanges version has ended up on the SPB. However, the changes addressing the review are complete.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

623a7a8 version bump

FelixErnst commented 5 years ago

@lshep For the last build a report did not get posted. Thanks for any help in advance.

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

Kayla-Morrell commented 5 years ago

Hello @FelixErnst,

Thank you for the work done on your package. I have looked over the changes made and have just a few more requirements/suggestions before accepting your package. See the secondary review below and comment back here when you are ready for me to look it over again.

General Package Developement

R CMD check

[ ] REQUIRED: There is an error when running R CMD check. Below is the error:

* checking examples ... ERROR
Running examples in ‘Structstrings-Ex.R’ failed
The error most likely occurred in:

> ### Name: Structstrings-internals
> ### Title: Structstrings internals
> ### Aliases: Structstrings-internals DOTBRACKET_CHAR_VALUES
> ###   DOTBRACKET_ALPHABET STRUCTURE_NEUTRAL_CHR STRUCTURE_OPEN_CHR
> ###   STRUCTURE_CLOSE_CHR [<-,DotBracketDataFrame-method
> ###   colnames<-,CompressedSplitDotBracketDataFrameList-method
> ### Keywords: datasets
>
> ### ** Examples
>
> DOTBRACKET_CHAR_VALUES
[1]  40  41  46  60  62  91  93 123 125
> DOTBRACKET_ALPHABET
[1] "(" ")" "." "<" ">" "[" "]" "{" "}"
> STRUCTURE_NEUTRAL_CHR
[1] "."
> STRUCTURE_OPEN_CHR
[1] "\\(" "<"   "\\[" "\\{"
> STRUCTURE_CLOSE_CHR
[1] "\\)" ">"   "\\]" "\\}"
>
> # the replace method for a DotBracketDataFrame had to be reimplemented
> # because of the requirement of columns for a DotBracketDataFrameList and
> # DotBracketDataFrame
> data("dbs", package = "Structstrings", envir = environment())
> dbdfl <- getBasePairing(dbs)
> # Elements are returned as DotBracketDataFrames
> dbdf <- dbdfl[[1]]
> dbdfl[[1]] <- dbdf
Error: C stack usage  7972064 is too close to the limit
Execution halted

vignette

[ ] SUGGESTION: Thank you for adding session info to the end of the vignette. However, you can just use SessionInfo() at the end of the vignette as it comes from the utils package. This way you won't have to add sessioninfo as a suggested package in your DESCRIPTION file.

man pages

DotBracketStringSet-io

[ ] REQUIRED: Line 50, I believe 'has' should be 'have'.

Best, Kayla

FelixErnst commented 5 years ago

Hi @Kayla-Morrell,

Thanks you for the second review.

General Package Developement

R CMD check the error you mentioned is not reported in the last build report. If you installed Structstrings locally and this is an error from that installation, please make sure to install the latest IRanges version using install_github("Bioconductor/IRanges"). The error is resolved there (see build report from yesterday).

vignette

done.

man pages

Thanks for catching that. Much appreciated.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

acccf16 simplification of vignette/DESCRIPTION and gramar ...

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

bioc-issue-bot commented 5 years ago

Your package has been accepted. It will be added to the Bioconductor Git repository and nightly builds. Additional information will be posed to this issue in the next several days.

Thank you for contributing to Bioconductor!

FelixErnst commented 5 years ago

@Kayla-Morrell Thank you for the review. Much appreciated!

mtmorgan commented 5 years ago

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/FelixErnst.keys is not empty), then no further steps are required. Otherwise, do the following:

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

https://bioconductor.org/packages/Structstrings

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.

Bioconductor / Contributions

Structstrings #1015

General Package Development

vignette

man pages

R code

General Package Development

vignette

man pages

R code

slot access:

unexported functions

General Package Developement

vignette

man pages

General Package Developement

vignette

man pages