Skip to content

Commit 035b878

Browse files
committed
add tests, coverage, better cache control
1 parent 0bc498e commit 035b878

19 files changed

+362
-127
lines changed

.Rbuildignore

+1
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@
99
^LICENSE$
1010
^.*\.Rproj$
1111
^\.Rproj\.user$
12+
^codecov\.yml$

.github/workflows/check-bioc.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ name: R4.1-bioc3.13
3434
## Note that you can always run a GHA test without the cache by using the word
3535
## "/nocache" in the commit message.
3636
env:
37-
has_testthat: 'false'
38-
run_covr: 'false'
37+
has_testthat: 'true'
38+
run_covr: 'true'
3939
run_pkgdown: 'true'
4040
has_RUnit: 'false'
4141
cache-version: 'cache-v1'

DESCRIPTION

+14-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Package: mistyR
22
Type: Package
33
Title: Multiview Intercellular SpaTial modeling framework
4-
Version: 0.99.10
4+
Version: 0.99.11
55
Authors@R: c(
66
person(given = "Jovan",
77
family = "Tanevski",
@@ -39,8 +39,19 @@ VignetteBuilder: knitr
3939
Imports: assertthat, caret, deldir, digest, distances, dplyr, filelock,
4040
furrr (>= 0.2.0), ggplot2, MASS, purrr, ranger, readr, rlang, rlist,
4141
R.utils, stats, stringr, tibble, tidyr, withr
42-
Suggests: BiocStyle, future, igraph, knitr, Matrix, progeny, rmarkdown,
43-
sctransform, SingleCellExperiment, SpatialExperiment, SummarizedExperiment,
42+
Suggests:
43+
covr,
44+
BiocStyle,
45+
future,
46+
igraph,
47+
knitr,
48+
Matrix,
49+
progeny,
50+
rmarkdown,
51+
sctransform,
52+
SingleCellExperiment,
53+
SpatialExperiment,
54+
SummarizedExperiment,
4455
testthat (>= 3.0.0)
4556
RoxygenNote: 7.1.1
4657
Config/testthat/edition: 3

NEWS.md

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
# mistyR 0.99.10
1+
# mistyR 0.99.11
22

33
- Fixed a bug in Nystrom approximation for creating paraview.
4-
- Added tests for view composition.
4+
- Added a suite of tests with high coverage.
5+
- Cleaner cache control.
56

67
# mistyR 0.99.9
78

R/misty.R

+13-20
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ run_misty <- function(views, results.folder = "results", seed = 42,
7474
dir.create(normalized.results.folder, recursive = TRUE)
7575
}
7676

77+
on.exit(sweep_cache())
78+
7779
view.abbrev <- views %>%
7880
rlist::list.remove(c("misty.uniqueid")) %>%
7981
purrr::map_chr(~ .x[["abbrev"]])
@@ -100,7 +102,7 @@ run_misty <- function(views, results.folder = "results", seed = 42,
100102
normalized.results.folder, .Platform$file.sep,
101103
"coefficients.txt.lock"
102104
)
103-
on.exit(file.remove(coef.lock))
105+
on.exit(file.remove(coef.lock), add = TRUE)
104106

105107
if (!append) {
106108
current.lock <- filelock::lock(coef.lock)
@@ -188,15 +190,19 @@ run_misty <- function(views, results.folder = "results", seed = 42,
188190
dplyr::mutate_if(~ sum(. < 0) > 0, ~ pmax(., 0))
189191
performance.summary <- c(
190192
performance.estimate %>% colMeans(),
191-
tryCatch(stats::t.test(performance.estimate %>% dplyr::pull(.data$intra.RMSE),
192-
performance.estimate %>% dplyr::pull(.data$multi.RMSE),
193-
alternative = "greater"
193+
tryCatch(stats::t.test(performance.estimate %>%
194+
dplyr::pull(.data$intra.RMSE),
195+
performance.estimate %>%
196+
dplyr::pull(.data$multi.RMSE),
197+
alternative = "greater"
194198
)$p.value, error = function(e) {
195199
1
196200
}),
197-
tryCatch(stats::t.test(performance.estimate %>% dplyr::pull(.data$intra.R2),
198-
performance.estimate %>% dplyr::pull(.data$multi.R2),
199-
alternative = "less"
201+
tryCatch(stats::t.test(performance.estimate %>%
202+
dplyr::pull(.data$intra.R2),
203+
performance.estimate %>%
204+
dplyr::pull(.data$multi.R2),
205+
alternative = "less"
200206
)$p.value, error = function(e) {
201207
1
202208
})
@@ -211,18 +217,5 @@ run_misty <- function(views, results.folder = "results", seed = 42,
211217
return(target)
212218
}, .progress = TRUE, .options = furrr::furrr_options(seed = TRUE))
213219

214-
if (!cached) {
215-
cache.location <- R.utils::getAbsolutePath(paste0(
216-
".misty.temp", .Platform$file.sep,
217-
views[["misty.uniqueid"]]
218-
))
219-
if (length(list.files(cache.location)) == 0) {
220-
clear_cache(views[["misty.uniqueid"]])
221-
}
222-
if (length(list.files(R.utils::getAbsolutePath(".misty.temp"))) == 0) {
223-
clear_cache()
224-
}
225-
}
226-
227220
return(normalized.results.folder)
228221
}

R/models.R

+6-5
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,16 @@
1717
#'
1818
#' @return A list containing the trained meta-model, a list of trained
1919
#' view-specific models and performance estimates.
20-
#'
20+
#'
2121
#' @noRd
2222
build_model <- function(views, target, seed = 42, cv.folds = 10, cached = FALSE,
2323
...) {
24-
2524
cache.location <- R.utils::getAbsolutePath(paste0(
2625
".misty.temp", .Platform$file.sep,
2726
views[["misty.uniqueid"]]
2827
))
2928

30-
if (!dir.exists(cache.location)) {
29+
if (cached && !dir.exists(cache.location)) {
3130
dir.create(cache.location, recursive = TRUE, showWarnings = TRUE)
3231
}
3332

@@ -90,8 +89,10 @@ build_model <- function(views, target, seed = 42, cv.folds = 10, cached = FALSE,
9089
)
9190

9291
# cv performance estimate
93-
test.folds <- withr::with_seed(seed,
94-
caret::createFolds(target.vector, k = cv.folds))
92+
test.folds <- withr::with_seed(
93+
seed,
94+
caret::createFolds(target.vector, k = cv.folds)
95+
)
9596

9697
intra.view.only <-
9798
model.views[["intraview"]]$predictions %>%

R/utils.R

+57-35
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,6 @@
11
# mistyR utility functions
22
# Copyright (c) 2020 Jovan Tanevski <[email protected]>
33

4-
#' Clear cached objects
5-
#'
6-
#' Purge the cache or clear the cached objects for a single sample.
7-
#'
8-
#' The cached objects are removed from disk and cannot be retrieved. Whenever
9-
#' possible specifying an \code{id} is reccomended. If \code{id = NULL} all
10-
#' contents of the folder \file{.misty.temp} will be removed.
11-
#'
12-
#' @param id the unique id of the sample.
13-
#'
14-
#' @return None (\code{NULL})
15-
#'
16-
#' @examples
17-
#' clear_cache("b98ad35f4e671871cba35f2155228612")
18-
#'
19-
#' clear_cache()
20-
#' @export
21-
clear_cache <- function(id = NULL) {
22-
cache.folder <- R.utils::getAbsolutePath(".misty.temp")
23-
if (is.null(id)) {
24-
if (!unlink(cache.folder, recursive = TRUE) == 0) {
25-
warning("Failed to clear cache.")
26-
}
27-
} else {
28-
if (!unlink(paste0(
29-
cache.folder, .Platform$file.sep, id
30-
), recursive = TRUE) == 0) {
31-
warning("Failed to clear cache.")
32-
}
33-
}
34-
}
35-
36-
374
#' Collect and aggregate results
385
#'
396
#' Collect and aggregate performance, contribution and importance estimations
@@ -187,7 +154,8 @@ collect_results <- function(folders) {
187154
dplyr::filter(sample == !!sample, view == paste0("p.", !!view)) %>%
188155
dplyr::mutate(value = 1 - .data$value)
189156

190-
# importances are standardized for each target an multiplied by 1-pval(view)
157+
# importances are standardized for each target
158+
# and multiplied by 1-pval(view)
191159
all.importances %>%
192160
purrr::imap_dfc(~
193161
tibble::tibble(feature = features, zero.imp = 0) %>%
@@ -249,7 +217,8 @@ aggregate_results_subset <- function(misty.results, folders) {
249217

250218
normalized.folders <- R.utils::getAbsolutePath(folders)
251219
# check if folders are in names of misty.results
252-
assertthat::assert_that(all(normalized.folders %in% names(misty.results$importances)),
220+
assertthat::assert_that(all(normalized.folders %in%
221+
names(misty.results$importances)),
253222
msg = "The provided results list doesn't contain information about some of
254223
the requested result folders. Consider using collect_results()."
255224
)
@@ -271,3 +240,56 @@ aggregate_results_subset <- function(misty.results, folders) {
271240

272241
return(misty.results)
273242
}
243+
244+
#' Clear cached objects
245+
#'
246+
#' Purge the cache or clear the cached objects for a single sample.
247+
#'
248+
#' The cached objects are removed from disk and cannot be retrieved. Whenever
249+
#' possible specifying an \code{id} is reccomended. If \code{id = NULL} all
250+
#' contents of the folder \file{.misty.temp} will be removed.
251+
#'
252+
#' @param id the unique id of the sample.
253+
#'
254+
#' @return None (\code{NULL})
255+
#'
256+
#' @examples
257+
#' clear_cache("b98ad35f4e671871cba35f2155228612")
258+
#'
259+
#' clear_cache()
260+
#' @export
261+
clear_cache <- function(id = NULL) {
262+
cache.folder <- R.utils::getAbsolutePath(".misty.temp")
263+
if (is.null(id)) {
264+
if (!unlink(cache.folder, recursive = TRUE) == 0) {
265+
warning("Failed to clear cache.")
266+
}
267+
} else {
268+
if (!unlink(paste0(
269+
cache.folder, .Platform$file.sep, id
270+
), recursive = TRUE) == 0) {
271+
warning("Failed to clear cache.")
272+
}
273+
}
274+
}
275+
276+
#' Removes empty cache folders.
277+
#'
278+
#' @return None (\code{NULL})
279+
#'
280+
#' @noRd
281+
sweep_cache <- function() {
282+
cache.folder <- R.utils::getAbsolutePath(".misty.temp")
283+
if (dir.exists(cache.folder)) {
284+
list.files(cache.folder, full.names = TRUE) %>%
285+
purrr::walk(function(path) {
286+
if (length(list.files(path)) == 0) {
287+
unlink(path, recursive = TRUE)
288+
}
289+
})
290+
291+
if (length(list.files(cache.folder, full.names = TRUE)) == 0) {
292+
clear_cache()
293+
}
294+
}
295+
}

0 commit comments

Comments
 (0)