Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

msnset se coercions #284

Merged
merged 5 commits into from
Dec 17, 2017
Merged

msnset se coercions #284

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
11294e0
msnset se coercions
arnesmits Dec 8, 2017
6c60f19
changes regarding Sebastian's comments
arnesmits Dec 8, 2017
9467796
transfer to setAs
arnesmits Dec 11, 2017
b96dc91
Update DESCRIPTION
LaurentGatto Dec 11, 2017
9fe34ff
MSnSet slots to SE metadata
arnesmits Dec 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions R/msnset2se.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#' SummarizedExperiment to MSnSet object conversion
#'
#' \code{se2msnset} generates an MSnSet object from an SummarizedExperiment object.
#'
#' @param se SummarizedExperiment,
#' Object which will be turned into an MSnSet object.
#' @return MSnSet object.
#' @export
se2msnset <- function(se) {
# Check input
assertthat::assert_that(inherits(se, "SummarizedExperiment"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we don't use assertthat for evaluation of input variables. While it is very convenient it don't think we are accepting a new dependency for just an easy if (inherits(se, "SummarizedExperiment")) check. BTW: assertthat is not listed in the DESCRIPTION file.


# Extract expression, feature and pheno data
raw <- SummarizedExperiment::assay(se)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we also have no dependency on SummarizedExperiment. That means you have to add SummarizedExperiment to the DESCRIPTION file. I would suggest to add it into Suggests: and use if (requireNamespace("SummarizedExperiment")) here to check whether it is available or not.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The :: should not necessary. You only need it if you have two functions with the same name in two imported packages. Beside adding SummarizedExperiment to the DESCRIPTION file you have to import the class and the methods you use via NAMESPACE: importClassesFrom(SummarizedExperiment, SummarizedExperiment) and importFrom(SummarizedExperiment, assay, colData, rowData).

feat_data <- data.frame(SummarizedExperiment::rowData(se))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use snake_case. We try to follow the bioc coding style and use camelCase

rownames(feat_data) <- se@NAMES
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not access slots directly. Why not use names(se) here? I am not very familiar with SummarizedExperiment yet but are you sure that rownames(rowData(se)) is never set?

pheno_data <- data.frame(SummarizedExperiment::colData(se))

# Generate MSnSet
msnset <- MSnbase::MSnSet(exprs = as.matrix(raw),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to use MSnbase:: because there are no other MSnSets around and the namespace of the package is always used first.

pData = Biobase::AnnotatedDataFrame(pheno_data),
fData = Biobase::AnnotatedDataFrame(feat_data))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need Biobase:: as well. We are depending on Biobase so AnnotatedDataFrame is available without ::.

return(msnset)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return isn't necessary here. The last object created in the function would be returned automatically. So msnset <- is also not necessary.

}

#' MSnSet to SummarizedExperiment object conversion
#'
#' \code{msnset2se} generates an SummarizedExperiment object from an MSnSet object.
#'
#' @param msnset MSnSet object,
#' Object which will be turned into an SummarizedExperiment object.
#' @return SummarizedExperiment object.
#' @export
msnset2se <- function(msnset) {
# Check input
assertthat::assert_that(inherits(msnset, "MSnSet"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above regarding assertthat.


# Extract assay, rowData and colData
raw <- Biobase::exprs(msnset)
row_data <- Biobase::fData(msnset)
col_data <- Biobase::pData(msnset)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need Biobase:: here as well.


# Generate SE
se <- SummarizedExperiment::SummarizedExperiment(assays = as.matrix(raw),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If SummarizedExperiment was attached by requireNamespace (which is missing here) the :: operator isn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added requireNamespace, however I still need the :: operator.

rowData = row_data,
colData = col_data)
return(se)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return not needed.

}
26 changes: 26 additions & 0 deletions tests/testthat/test_msnset_se_coercions.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
context("msnset - se conversion methods")

test_that("msnset2se conversion", {
data(msnset, package = "MSnbase")
se <- msnset2se(msnset)

expect_is(se, "SummarizedExperiment")
expect_equal(colnames(msnset), colnames(se))
expect_equal(rownames(msnset), rownames(se))
expect_equal(Biobase::exprs(msnset), SummarizedExperiment::assay(se))
expect_equal(Biobase::fData(msnset), data.frame(SummarizedExperiment::rowData(se), row.names = se@NAMES))
expect_equal(Biobase::pData(msnset), data.frame(SummarizedExperiment::colData(se)))
})

test_that("se2msnset conversion", {
data(msnset, package = "MSnbase")
se <- msnset2se(msnset)
msnset_back <- se2msnset(se)

expect_is(msnset_back, "MSnSet")
expect_equal(colnames(msnset_back), colnames(se))
expect_equal(rownames(msnset_back), rownames(se))
expect_equal(Biobase::exprs(msnset_back), SummarizedExperiment::assay(se))
expect_equal(Biobase::fData(msnset_back), data.frame(SummarizedExperiment::rowData(se), row.names = se@NAMES))
expect_equal(Biobase::pData(msnset_back), data.frame(SummarizedExperiment::colData(se)))
})