Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

(inactive) inferrnal - wrapper for Infernal RNA sequence analysis programs #1748

Closed brendanf closed 3 years ago

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

bioc-issue-bot commented 4 years ago

Hi @brendanf

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: inferrnal
Title: Interface to Call Programs from Infernal RNA Covariance Model Package
Version: 0.99.5
Authors@R: 
    person(given = "Brendan",
 family = "Furneaux",
 role = c("aut", "cre"),
 email = "brendan.furneaux@gmail.com",
 comment = c(ORCID = "0000-0003-3522-7363"))
Description: A thin wrapper around the Infernal package to search for matches to
    a covariance model in RNA sequences, or to align RNA sequences to a 
    covariance model. Infernal is not included in the package and needs to be
    installed independently.
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
SystemRequirements: Infernal
URL: https://github.com/brendanf/inferrnal
BugReports: https://github.com/brendanf/inferrnal/issues
Imports: 
    assertthat,
    Biostrings,
    S4Vectors,
    methods,
    stats
RoxygenNote: 7.1.0
biocViews:
    SequenceMatching,
    ThirdPartyClient,
    Metagenomics,
    HiddenMarkovModel,
    DNASeq,
    RNASeq,
    Annotation,
    Alignment,
    MultipleSequenceAlignment
Suggests: 
    testthat (>= 2.1.0),
    knitr,
    rmarkdown,
    covr,
    ShortRead
VignetteBuilder: knitr
bioc-issue-bot commented 4 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 4 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: "skipped, 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/inferrnal to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

brendanf commented 4 years ago

Because the package interfaces with Infernal, tests and vignette building will fail if Infernal is not present. (This means it cannot work on Windows.) Is there a way to specify the added requirement to the automated build system?

brendanf commented 4 years ago

(Infernal is available as an apt, conda, or homebrew package, as well as binaries for linux and osx).

Kayla-Morrell commented 3 years ago

@brendanf - Sorry for the late reply, here is what can be done for the 2 issues raised.

Windows issue: If you would like to not support Windows (since Infernal is not supported on Windows) you can do so. You should be aware though of the large portion of the Bioconductor community that will not be able to use your package. If this is something you wish to do you can include a .BBSoptions file in the top directory of the package and inside this file should be the following command UnsupportedPlatforms: win. Keep in mind that both the file name and command are case sensitive.

Infernal requirement: The first thing is to be sure that Infernal is listed as a SystemRequirement in the DESCRIPTION file, which you have. The next thing we require is a top level INSTALL file that provides the command for the system requirement to be installed for each platform. Whether that be sudo, pip, brew, we just need instructions for our build system admins to know what commands to run to install the requirement. Once that file is added and pushed they can go ahead and add the requirement.

Hope this helps, let me know if you have any further questions.

Best, Kayla

brendanf commented 3 years ago

@Kayla-Morrell Thanks. If I understand correctly, the INSTALL file is just human-readable?

Kayla-Morrell commented 3 years ago

@brendanf - yes, that's correct.

brendanf commented 3 years ago

OK, I made those changes and pushed to git@git.bioconductor.org:packages/inferrnal.

Kayla-Morrell commented 3 years ago

Great - we will work on getting that added and I'll let you know when it's ready.

Kayla-Morrell commented 3 years ago

@hpages - This is the package that requires the Infernal installed on the builders. Thanks!

hpages commented 3 years ago

@Kayla-Morrell @brendanf I installed Infernal on the Linux and Mac builders, following the instructions in the INSTALL file. Note that this file is useful not only for us, build system maintainers, but also for the users of the inferrnal package. So thanks @brendanf for providing it.

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

Kayla-Morrell commented 3 years ago

@brendanf - Since Infernal was installed on the builders, I went ahead and kicked off a manual build so you could get a proper build report.

Best, Kayla

brendanf commented 3 years ago

I've removed the .Rproj file and subscribed to the mailing list, so it should pass BiocCheckGitClone and BiocCheck without errors next time.

Kayla-Morrell commented 3 years ago

Make sure you push a version bump to ensure a new build is triggered (see here for more help).

bioc-issue-bot commented 3 years ago

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

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

Kayla-Morrell commented 3 years ago

@brendanf -

Thank you for submitting to Bioconductor. Please see the initial review of the package below. The required changes must be made while the suggested changes do not have to be (though we strongly encourage them). Comment back here with updates that have been made and when the package is ready for a re-review.

General package development

DESCRIPTION

Data

Vignette

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

Best, Kayla

brendanf commented 3 years ago

@Kayla-Morrell Thanks for the review. There are a few items that I have questions/comments about:

Including ShortRead in Imports:

The only functionality that requires the ShortRead package is accepting input sequences as objects of class ShortRead. That is there as a convenience for users who already have their sequences in this format. In that case, they obviously have the ShortRead package installed. I would prefer not to require the package for users who are not using it anyway. I can further gate that code using if (!requireNamespace("ShortRead", quietly=TRUE)) stop("Received a ShortRead object but the ShortRead package is unavailable") or something similar. Would that be acceptable?

README.Rmd

This is the source file for README.md. Any changes are made to README.Rmd, which is then rendered to make README.md. README.Rmd is in .Rbuildignore, so it is not included in the built package. Is it really unacceptable to have this file in the git repository?

codecov.yaml

This is settings for automatic calculation of test coverage on Travis, the results of which are included as badges in README.md. Like README.Rmd, it is excluded from the built package using .Rbuildignore.

Kayla-Morrell commented 3 years ago

@brendanf -

For the ShortRead import, it would be fine not to require it but the code you suggested should be implemented.

Thank you for explaining the README.Rmd and codecov.yaml files. These are fine to keep as they are in the .Rbuildignore file.

Kayla-Morrell commented 3 years ago

@brendanf - I was wondering if there has been any progress on the points made in my review. We typically like to see changes within a 2-3 week time frame to be sure the package is actively being worked on.

brendanf commented 3 years ago

@Kayla-Morrell - I had addressed most of your points prior to the holidays, but did not push until just now. I believe the only thing remaining is the data documentation.

Kayla-Morrell commented 3 years ago

@brendanf - It has been over a month now since I last checked in with you. If there are no changes pushed and a new build report produced within in the next couple days I'll be closing this issue for inactivity.

Kayla-Morrell commented 3 years ago

@brendanf - I am closing this package for inactivity. We normally like to see changes to a package within a few weeks to know they are actively being worked on. When you have implemented changes and are ready for a re-review, please comment back here to ask that the issue be reopened.

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

brendanf commented 3 years ago

Sorry for the delays! I'm working on revising the article, which will also help in generating the data citations. As you say, I'll reopen when it's ready.