regsplice

[lmweber]

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

• Repository: https://github.com/lmweber/regsplice

Confirm the following by editing each check box to '[x]'

• I understand that by submitting my package to Bioconductor,
  the package source and all review commentary are visible to the
  general public.
• I have read the Bioconductor Package Submission
  instructions. My package is consistent with the Bioconductor
  Package Guidelines.
• My package addresses statistical or bioinformatic issues related
  to the analysis and comprehension of high throughput genomic data.
• 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:

• The 'devel' branch for new packages and features.
• The stable 'release' branch, made available every six
  months, for bug fixes.
• Bioconductor version control using Subversion
  (optionally via GitHub).

For help with submitting your package, please subscribe and post questions
to the bioc-devel mailing list.

[bioc-issue-bot]

Hi @lmweber

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: regsplice
Version: 0.99.0
Title: Lasso-Based Methods for Detection of Differential Exon Usage
Description: Statistical methods for detection of differential exon usage
    in RNA-seq and exon microarray data sets, using L1 regularization (lasso) to
    improve power.
Authors@R: person("Lukas M.", "Weber", email = "lukmweber@gmail.com", role = c("aut", "cre"))
Author: Lukas M. Weber [aut, cre]
Maintainer: Lukas M. Weber <lukmweber@gmail.com>
URL: https://github.com/lmweber/regsplice
BugReports: https://github.com/lmweber/regsplice/issues
License: MIT + file LICENSE
LazyData: true
Imports:
    glmnet,
    SummarizedExperiment,
    S4Vectors,
    BiocParallel,
    limma,
    edgeR,
    stats,
    utils, 
    methods
Suggests:
    testthat,
    BiocStyle,
    knitr,
    rmarkdown
VignetteBuilder: knitr
RoxygenNote: 5.0.1
biocViews: AlternativeSplicing, DifferentialExpression, DifferentialSplicing,
    Sequencing, RNASeq, Microarray, ExonArray, ExperimentalDesign, Software
Collate:
    'class_RegspliceResults.R'
    'class_RegspliceData.R'
    'LR_tests.R'
    'create_design_matrix.R'
    'filter_low_counts.R'
    'filter_zeros.R'
    'fitting_functions_multiple_genes.R'
    'fitting_functions_single_gene.R'
    'helper_functions.R'
    'initialize_results.R'
    'regsplice_wrapper.R'
    'run_normalization.R'
    'run_voom.R'
    'summary_table.R'

[bioc-issue-bot]

Your package has been approved for building. Your package is
now submitted to our queue.

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]

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 following build report for more details:

http://bioconductor.org/spb_reports/regsplice_buildreport_20160922144958.html

[lawremi]

It would be nice if the top-level regsplice() accepted a SummarizedExperiment as input, presumably from an upstream call to summarizeOverlaps(). I like how regsplice already has a SummarizedExperiment-based class for its internal workflow. Doing the same for the primary entry point would enable interoperability with other pakages.

[lmweber]

Thanks for this suggestion @lawremi , yes this is a good idea. I'll get onto it.

[lmweber]

Hi @LiNk-NY @lawremi , can we still expect review comments before the Wednesday deadline? I will be on a plane on Tues/Wed so may be unable to respond in time if we get comments at the last minute. It will be disappointing to miss out on this Bioconductor release if we don't have time to respond. I do appreciate the time you are taking to review our package and hope to hear back soon. Best regards, Lukas

[LiNk-NY]

Hi @lmweber,
Sorry for the delay. I'm reviewing your package now.
Thank you for your patience.
Marcel

[LiNk-NY]

Hi @lmweber,
Thank you for submitting your package to Bioconductor!
Please see the package review below:

---

Overall

The documentation of your package looks descriptive and organized. You make use of the SummarizedExperiment class and add some changes with your class.

User interface

• No need to provide a message when subsetting (i.e., "subsetting rows by *") . The user is expected to understand what is taking place when subsetting your object.
• Subsetting by columns? Something like Y[, 1:2] doesn't seem to work. You would want to keep the method consistency if your object class is based on a SummarizedExperiment.
• Use of @ is discouraged. We would prefer that you use accessor functions to avoid the use of @ symbol (particularly for the RegspliceResults object).
• Your internal functions should start with a period.
• You could also support DataFrame class objects as input for counts.
• Shouldn't regsplice use RegspliceData as input? Some of the arguments are the same in both and you create that object at the start of the function anyway.

Grammar

• We try to avoid using the use of underscores in function names. Please use camelCase instead.
• You might want to add a check for condition length to equal sample count in the constructor function.
• Avoid using 1:number or 1:length(vector) try seq_len(number) or seq_along(vector)
• Please change your data argument name to something like RegspliceData so that the user knows what is expected (data is also a function in utils).
• Minor point: I don't think you need multiple @importFrom directives as they will resolve to one call in the NAMESPACE.

On the note you have for one of your functions, have you tried contacting the maintainer of glmnet for a bug fix?

Thank you for submitting to Bioconductor. We hope to see the appropriate changes and accepted to Bioconductor.

Regards,
Marcel

[bioc-issue-bot]

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

a8f6e84 don't need .tar.gz file
d7c25da accept SummarizedExperiment input objects
b2b1b77 update vignette
5a8543f add accessor functions for RegspliceResults object...
63efe34 use 'seq_len' and 'seq_along' instead of '1:length...
1e011e1 update NEWS
6ad965e consistency between accessor function names and sl...
7d930a9 change workflow so RegspliceData objects are creat...
02f5c37 fix bug when 'i' missing in RegspliceData subsetti...
5817a4a remove messages when row-subsetting RegspliceData ...
76d4443 accept input 'counts' as DataFrame
4d0d7cb internal function names start with dot
075e908 check number of columns in 'counts'
4fa43e7 change argument names to 'rs_data' and 'rs_results...
86efec7 switch function names to camelCase
662b83f version bump

[bioc-issue-bot]

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 following build report for more details:

http://bioconductor.org/spb_reports/regsplice_buildreport_20161011005253.html

[lmweber]

Hi Marcel,

Thank you for your review. These are all helpful comments.

I have added these changes in the updated version just submitted. More details on each point are below.

---

User interface

• No need to provide a message when subsetting (i.e., "subsetting rows by *") . The user is expected to understand what is taking place when subsetting your object.

Done. I have removed these messages.

• Subsetting by columns? Something like Y[, 1:2] doesn't seem to work. You would want to keep the method consistency if your object class is based on a SummarizedExperiment.

This was a bug, which occurred when the argument "i" was missing in a subsetting call. This is now fixed, so subsetting should be consistent with SummarizedExperiment.

• Use of @ is discouraged. We would prefer that you use accessor functions to avoid the use of @ symbol (particularly for the RegspliceResults object).

Done. I have added accessor functions for all the main parts of the RegspliceResults objects. The examples in the vignette now all use accessor functions instead of @.

• Your internal functions should start with a period.

Done.

• You could also support DataFrame class objects as input for counts.

Done.

• Shouldn't regsplice use RegspliceData as input? Some of the arguments are the same in both and you create that object at the start of the function anyway.

I have re-structured the workflow so that the RegspliceData constructor is now always called explicitly. In a typical workflow, the user will now first create a data object using the RegspliceData() constructor, and then run the analysis pipeline with the regsplice() wrapper function. This is a cleaner separation than I had before. In addition, I have modified the constructor to also accept SummarizedExperiment input objects, as per @lawremi 's suggestion.

Grammar

• We try to avoid using the use of underscores in function names. Please use camelCase instead.

Done. Function names now use camelCase.

• You might want to add a check for condition length to equal sample count in the constructor function.

Done.

• Avoid using 1:number or 1:length(vector) try seq_len(number) or seq_along(vector)

Done.

• Please change your data argument name to something like RegspliceData so that the user knows what is expected (data is also a function in utils).

Done. I have changed the data arguments to "rs_data", and the results arguments to "rs_results". This is now consistent throughout the package.

• Minor point: I don't think you need multiple @importFrom directives as they will resolve to one call in the NAMESPACE.

I started removing some of these @importFrom directives, but decided to leave them in for now. I think it helps readability to have all the imports above each function block, and they don't seem to cause any problems. However I'll think about whether there is a good place to collect some of them.

• On the note you have for one of your functions, have you tried contacting the maintainer of glmnet for a bug fix?

Yes, we contacted the package maintainer and submitted a bug report. They have added this to their list of updates, and it will be included in a future version. However the version currently on CRAN still has this issue, so I have included the note for now.

---

Thanks again and best regards,
Lukas

[lmweber]

Hi @LiNk-NY @lawremi , sorry to keep sending messages. I noticed the release schedule mentions "No new packages added to BioC release roster" from today (Wed 12 Oct). Will we still be considered for inclusion in this release, or is it now too late? Best regards, Lukas

[LiNk-NY]

Hi @lmweber,
It looks like you still have generic and method names with under_score characters.

Regards,
Marcel

[lmweber]

Thanks for the reply. I think these are the accessors for the RegspliceResults objects, i.e. gene_IDs(), p_vals(), p_adj(), LR_stats(), and df_tests(). I left these since I thought it would be better to keep them consistent with the slot names that they are accessing (the slot names use underscores). However I'll update them now to camelCase if this is a strict requirement.

[lmweber]

Thank you for accepting the package, glad we made it! Thanks again for your time @LiNk-NY .