ASpediaFI

[nachoryu]

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

• Repository: https://github.com/nachoryu/ASpediaFI

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]

Hi @nachoryu

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: ASpediaFI
Type: Package
Title: ASpedia-FI: Functional Interaction Analysis of Alternative Splicing Events
Version: 0.99.0
Date: 2019-10-01
Author: Doyeong Yu
Maintainer: Doyeong Yu <nachoryu@ncc.re.kr>
Description: This package provides functionalities for a systematic and integrative analysis of alternative splicing events and their functional interactions. 
License: GPL-3
Encoding: UTF-8
biocViews: AlternativeSplicing, Annotation, Coverage, GeneExpression, GeneSetEnrichment, GraphAndNetwork, KEGG, Network, NetworkInference, Pathways, Reactome, Transcription, Sequencing, Visualization
LazyData: true
RoxygenNote: 6.1.1
Depends: R (>= 3.6.0), SummarizedExperiment, ROCR
Imports: BiocParallel, GenomicAlignments, GenomicFeatures, GenomicRanges, 
    IRanges, IVAS, Rsamtools, biomaRt, limma, S4Vectors, stats,
    DRaWR, GenomeInfoDb, Gviz, Matrix, dplyr, fgsea, reshape2, igraph,
    graphics, e1071, methods, rtracklayer, scales, grid, ggplot2, mGSZ
Suggests: knitr
VignetteBuilder: knitr

[bioc-issue-bot]

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]

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

ae61914 Added NEWS (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.

On one or more platforms, the build results were: "WARNINGS, 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]

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, 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]

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

f5de0ef Version bump (0.99.2)

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

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.

[bioc-issue-bot]

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

254b9a4 Package loading issue

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

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.

[bioc-issue-bot]

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

332e50f Fixed vignette issue

[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 build report for more details.

[nachoryu]

Hello @Kayla-Morrell,

the package has been built without errors and warnings on all platforms. I would like to know if it is ready for review.

Thanks,

Doyeong

[Kayla-Morrell]

Hello @nachoryu,

There is nothing you have to do at the moment. I'm working on the review and should have it posted in a couple days.

Best, Kayla

[Kayla-Morrell]

Hello @nachoryu,

I have a general question regarding your package. Is there a reason why you chose to create a reference class in your package? We normally prefer the use of S4 classes.

[nachoryu]

Hello @Kayla-Morrell,

the functions in this package have many arguments and options, and some inputs are used more than once throughout the workflow. For example, a GTF file is used for annotation of AS events, and also used for their visualization as a GRanges object after network anlaysis . Since a reference class is mutable and its methods directly belong to itself, I thought it will allow the user to take advantage of our package and the ASpediaFI workflow more conveniently.

Thanks,

Doyeong

[hpages]

Hi Doyeong,

I don't see any advantage of reference classes over regular S4 classes here, only unnecessary struggle for your users. Here is why:

copy-on-change semantic vs reference semantic

In R, objects should normally have a copy-on-change semantic, not a reference semantic. This is the standard semantic and that's what users are used to and are comfortable with. People coming from other programming languages are sometimes confused and seem to believe that this means that objects cannot be modified. This is a common misconception. Objects with a copy-on-change semantic can be modified via setters, and, more generally, via functions that take an object and return a modified copy of the object. From a user point-of-view this means using idioms like:

gtf(object) <- my_gtf  # setter

or:

object <- analyzeFI(object, ...)  # returns a modified copy of the object

People sometimes worry about performance. It is true that copy-on-change semantic means immutable objects, that is, objects that cannot be modified in-place. So of course trying to modify them generates copies, at least in theory. However, most of the time, like in the two examples above, and with carefully written code, R is smart enough to avoid these copies so the copy-on-change semantic will generally not affect performance.

But what's the benefit of copy-on-change semantic anyway?

The major benefit of sticking to that semantic is that the user can choose to replace the current object with the modified one:

object <- quantifyPSI(object, ...)  # replace current object with modified one
object <- analyzeFI(object, ...)    # replace again current object with modified one

or not:

object2 <- quantifyPSI(object, ...)
object3 <- analyzeFI(object2, ...)

With the latter, thanks to the copy-on-change semantic, object is guaranteed to not be touched/altered by the call to quantifyPSI() and object2 is guaranteed to not be touched/altered by the call to analyzeFI(). More generally speaking, no function call can alter the content of an object that has copy-on-change semantic. This is a good thing. This allows the user to keep various versions of the object in her session as she goes thru the various steps of the workflow.

Note that with reference objects, things are very different. It's still possible to keep various versions of the object as one walks thru the workflow e.g. with something like:

object2 <- object$copy()
object2$quantifyPSI(...)
object3 <- object2$copy()
object3$analyzeFI(...)

But most R users are not familiar with this approach and shouldn't be forced into it. In fact, most R users won't necessarily fully grasp the consequences of dealing with reference objects and so will likely do something like:

previous_object <- object
object$quantifyPSI(...)

with disastrous consequences! (Because they don't realize that the call to quantifyPSI() also modifies previous_object.)

Bottom line

R is not Python and, for the sake of your users, it's important that you understand and embrace the R way.

An unrelated issue

• While I was taking a quick look at ASpediaFI, I ran into the following error (this is code taken from the vignette):

  library(ASpediaFI)
  GSE114922.ASpediaFI <- ASpediaFI()
  gtf <- system.file("extdata/GRCh38.subset.gtf", package = "ASpediaFI")
  GSE114922.ASpediaFI$annotateASevents(gtf.file = gtf, num.cores = 1)
  bamMT <- system.file("extdata/GSM3167287.subset.bam", package = "ASpediaFI")
  bamWT <- system.file("extdata/GSM3167290.subset.bam", package = "ASpediaFI")
  GSE114922.ASpediaFI$samples <- data.frame(name = c("GSM3167287", "GSM3167290"),
                                            path = c(bamMT, bamWT),
                                            condition = c("MT", "WT"))
  GSE114922.ASpediaFI$quantifyPSI(read.type = "paired", read.length = 100,
                                  insert.size = 300, min.reads = 3, num.cores = 1)
  # Error in (function (classes, fdef, mtable)  :
  #   unable to find an inherited method for function 'readGAlignments' for signature '"integer"'

  After looking more into this, I realized that this happened because I didn't have global option stringsAsFactors set to FALSE. So it seems that your code assumes that the user has global option stringsAsFactors set to FALSE. This is not good. Your code should work independently of how the user has set global option stringsAsFactors.

• For the same reason it's not a good idea to set global option stringsAsFactors to FALSE at the beginning of your vignette (it makes your code "artificially" work i.e. it prevents R CMD build from catching errors that it would otherwise catch).

Hope this helps. Thanks for your submission!

Regards, H.

[bioc-issue-bot]

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

cbf1551 Implemented S4 Class

[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 build report for more details.

[nachoryu]

Hello @Kayla-Morrell and @hpages,

I made a change to implement S4 classes instead of reference class. Also, I fixed the issue from the vignette which @hpages mentioned.

Thanks,

Doyeong

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

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]

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

c37771d Replaced "1:" with "seq_len"

[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 build report for more details.

[Kayla-Morrell]

Hello @nachoryu -

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 but we strongly encourage them. Comment back here with updates that have been made and when the package is ready for a re-review. Please keep in mind that the deadline for accepting new packages into the Bioconductor 3.10 release is 10/23.

General package development

R CMD check

• [x] REQUIRED: Installed package size is 8.8Mb, please work on decreasing the size. The sub-directories that are 1Mb or more are:

  • R, 1.6Mb
  • data, 5.1 Mb
  • extdata, 1.3Mb

• [x] REQUIRED: Since you are using a standard license, the LICENSE.md file is not needed in the top directory and should be removed.

• [x] REQUIRED: There is a sysdata.rda file in the R/ directory. Is this file needed? If it is it should be in the data/ or inst/extdata directories with proper documentation. If not, then please remove it.

• [x] REQUIRED: What is the purpose of the file 'datalist' in data/?

DESCRIPTION

• [x] REQUIRED: 'LazyData' should be set to false. When this is true it can slow down the loading of packages with data.

• [x] SUGGESTION: We strongly encourage adding the 'BugReports:' tag with a relevant GitHub link where users can report Issues.

NAMESPACE

• [x] REQUIRED: Generally importFrom() is encouraged over importing an entire package, however if there are many functions from a single package, import() is okay. Please pick one or the other, it seems for GenomicFeatures you use both import() and importFrom().

• [x] REQUIRED: Lines 65-69, were these added by hand? All exporting/importing should be generated using roxygen2 documentation.

NEWS

• [x] REQUIRED: Thank you for adding a NEWS file, this can be very helpful to keep track of changes to code from one version to the next. However, the NEWS file doesn't seem to be formatted correctly. Please see the following link for formatting help: https://bioconductor.org/developers/package-guidelines/#news.

Data

• [x] REQUIRED: Be sure that all data in inst/extdata/ is needed. I didn't see any use of 'GSM3167287.subset.bam.bai' and 'GSM3167290.subset.bam.bai' in the vignettes or examples.

Vignettes

• [x] REQUIRED: The final section of the vignette, before the references should be a section called 'Session Info' that includes sessionInfo().

Man pages

ASpediaFI-class

• [x] REQUIRED: Man pages describing new classes must be very detailed on the structure and what type of information is stored. Be sure to include information about the slots of the class.

analyzeFI

• [x] REQUIRED: The last two lines of the example code for this man page are commented out. This line of code should be included in a \dontrun{} or \donttest{} section so that it is formatted correctly.

quantifyPSI

• [x] REQUIRED: The same goes for the last example in the example code. This code should not be commented out but instead use \dontrun{} or \donttest{}.

visualize

• [x] REQUIRED: We would prefer to have at least one example execute in the man pages, but since this function is also documented in the vignette it isn't absolutely necessary. However, please don't just comment out the code but instead use \dontrun{} or \donttest{}.

inst/script

• [x] REQUIRED: The scripts in this directory can vary. Most importantly if data was included in the inst/extdata/, a related script must be present in this directory documenting very clearly how the data was generated. It should include source urls and any important information regarding filtering or processing. It can be executible code, sudo code, or a text description. A user should be able to download and roughly reproduce the file or object that is present as data.

Unit tests

• [x] SUGGESTION: Consider adding unit tests. We strongly encourage them. See http://bioconductor.org/developers/how-to/unitTesting-guidelines/.

R code

• [x] SUGGESTION: Alternative splice sites gets shortened to ASS in many places throughout your code and in some messages. Unfortunately this short hand has a certain negative connotation and probably shouldn't be shown to the user where possible. I would say it could remain ASS in any internal functions but I would strongly encourage you to think of maybe shortening it to AS or some other acronym when exposing it to the user.

• [x] REQUIRED: Avoid the use of direct slot access with @ or slot(). Accessor methods should be created and utilized instead. For example, in analyzeFI.R lines 76-79 and annotateASevents.R lines 24 and 25. Please correct all instances of this in your code.

• [x] REQUIRED: Avoid sapply(); use vapply() instead. Found in files:

  • analyze.R, lines 119, 123, 233, 234, 236, 238, and 240
  • exportNetwork.R, line 24
  • filterEvent.R, line 56, 113, and 370
  • SplicingReads.R, line 118
  • visualizeNetwork.R, line 22

• [x] REQUIRED: Consider shorter lines, 215 lines are > 80 characters long.

• [x] REQUIRED: Consider multiples of 4 spaces for line indents, 733 lines are not.

Best, Kayla

[bioc-issue-bot]

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

07cf2ea Made changes to conform to Bioconductor guideline

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

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.

[bioc-issue-bot]

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

9306bff Made additional changes to conform to Bioconductor...

[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 build report for more details.

[nachoryu]

Hello @Kayla-Morrell,

Below are the changes made to address issues mentioned in your comments.

General package development

R CMD check

• Decreased the file sizes of datasets in the package.

• Removed the LICENSE.md file.

• sysdata.rda file contains human gene-gene interaction network and human pathway gene sets which are used internally in the function for functional interaction analysis. So, these datasets were saved in the sysdata.rda file, as explained in the Bioconductor package guidelines.

• The datalist file was removed.

DESCRIPTION

• Lazydata is now set to false.

• The BugReports tag was added.

NAMESPACE

• Removed ImportFrom entries for the GenomicFeatures package.

• Generated using roxygen2.

NEWS

• Formatted the NEWS file according to the Bioconductor guideline.

Data

• The bai file (which is BAM index file) is required for quantifying PSI profiles.

Vignettes

• Added the SessionInfo() section.

• Added slots section to the manual page.

Man pages

analyzeFI

• Included those lines in \dontrun.

quantifyPSI

• Included in \dontrun.

visualize

• Included in \dontrun.

inst/script

• Added README file about external data.

R code

• Shortened Alternative Splice Site to ALSS.

  • Implemented accessors.

• Replaced sapply with vapply.

• Made lines shorter except those in the vignette which cannot be shortened.

• Indented all lines except the one in the vignette.

Thank you,

Doyeong

[Kayla-Morrell]

@nachoryu - Thank you for making the necessary changes. Everything looks good now and I'd be more than happy to accept the package.

Best, Kayla

[bioc-issue-bot]

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!

[mtmorgan]

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

1. Add an SSH key to your github account
2. Submit your SSH key to Bioconductor

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

https://bioconductor.org/packages/ASpediaFI

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.