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.

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

Add support for S7 methods registered for external generics #586

Merged
merged 22 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
cf551c6
Initial work on S7 support for covr
jimhester Oct 29, 2024
682b17c
fix prop replacements in `traverse_S7_class`
t-kalinowski Nov 7, 2024
5d69e0e
handle multi-dispatch generics in `traverse_S7_generic()`
t-kalinowski Nov 7, 2024
659804d
add missing `test_path()` in tests
t-kalinowski Nov 7, 2024
8058180
update expected validator run counts in tests: 3 -> 4
t-kalinowski Nov 7, 2024
712211c
CamelCase S7 class name
t-kalinowski Nov 7, 2024
1599df7
Fix package-name / dir-name mismatch
t-kalinowski Nov 7, 2024
8687ed4
revert `test_path()` usage
t-kalinowski Nov 7, 2024
517a018
one more test pkgname fix
t-kalinowski Nov 7, 2024
459217f
add S7 to Suggests
t-kalinowski Nov 7, 2024
27038c2
Merge branch 'main' into S7
t-kalinowski Nov 7, 2024
4add15b
Revert "update expected validator run counts in tests: 3 -> 4"
t-kalinowski Nov 7, 2024
efbbd52
Use `S7::prop()` instead of `@` for R oldrel compat
t-kalinowski Nov 7, 2024
f228960
import backported `@` in TestS7 package
t-kalinowski Nov 7, 2024
085939a
stray `@` -> `S7::prop()` replacement
t-kalinowski Nov 8, 2024
6583c13
return of `test_path()`
t-kalinowski Nov 8, 2024
1636d12
Class validator runs 4 times, not 3
t-kalinowski Nov 8, 2024
59b1f7e
Install S7 >= 0.2.0
t-kalinowski Nov 8, 2024
580544d
better names in coverage report
t-kalinowski Nov 8, 2024
5d1cafa
Add support for S7 methods registered for external generics
t-kalinowski Nov 8, 2024
0aa82ed
enable `workflow_dispatch` for R-CMD-check action
t-kalinowski Nov 8, 2024
c55617e
Merge branch 'main' into S7-external-generics
t-kalinowski Nov 11, 2024
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
1 change: 1 addition & 0 deletions .github/workflows/R-CMD-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# check-standard.yaml is likely a better choice.
# usethis::use_github_action("check-standard") will install it.
on:
workflow_dispatch:
push:
branches: [main, master]
pull_request:
Expand Down
33 changes: 32 additions & 1 deletion R/S7.R
Original file line number Diff line number Diff line change
@@ -1,12 +1,27 @@
replacements_S7 <- function(env) {
unlist(recursive = FALSE, use.names = FALSE, eapply(env, all.names = TRUE,
bindings <- unlist(recursive = FALSE, use.names = FALSE, eapply(env, all.names = TRUE,
function(obj) {
if (inherits(obj, "S7_generic")) {
traverse_S7_generic(obj)
} else if (inherits(obj, "S7_class")) {
traverse_S7_class(obj)
}
}))

S7_methods_tbl <- attr(env[[".__S3MethodsTable__."]], "S7methods", exact = TRUE)
external_methods <- lapply(seq_along(S7_methods_tbl), function(i) {
entry <- S7_methods_tbl[[i]]
name <- external_generic_method_signature(entry$generic, entry$signature)

replacement(
# `name` is for informative printouts only.
# It is not used by covr, and does not need to be unique,
name = name,
env = entry,
target_value = entry$method)
})

c(bindings, external_methods)
}

traverse_S7_generic <- function(x) {
Expand Down Expand Up @@ -51,3 +66,19 @@ traverse_S7_class <- function(x) {
prop_fun_replacements
)
}


external_generic_method_signature <- function(generic, signature) {
# This function is a lightly modified copy of S7:::method_signature() for external generics
display_generic <- paste0(c(generic$package, generic$name), collapse = "::")
class_deparse <- asNamespace("S7")$class_deparse # not exported from S7 :/
single <- length(generic$dispatch_args) == 1
if (single) {
signature <- class_deparse(signature[[1]])
} else {
classes <- vapply(signature, class_deparse, "", USE.NAMES = FALSE)
signature <- paste0("list(", paste0(classes, collapse = ", "), ")")
}

sprintf("method(%s, %s)", display_generic, signature)
}
15 changes: 15 additions & 0 deletions tests/testthat/TestS7/R/foo.R
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,18 @@ method(inside, Range) <- function(x, y) {
# enable usage of <S7_object>@name in package code
#' @rawNamespace if (getRversion() < "4.3.0") importFrom("S7", "@")
NULL

# test external S3 generics
method(format, Range) <- function(x) {
sprintf("Range(%s, %s)", x@start, x@end)
}

testthat_print <- new_external_generic("testthat", "testthat_print", "x")
method(testthat_print, Range) <- function(x, ...) {
cat(format(x))
invisible(x)
}

.onLoad <- function(libname, pkgname) {
S7::methods_register()
}
8 changes: 8 additions & 0 deletions tests/testthat/TestS7/tests/testthat/test-foo.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,11 @@ test_that("Range works", {
expect_equal(x@length, 5)
expect_equal(x@end, 6)
})

test_that("Range methods work", {
x <- Range(1:10)
expect_equal(base::format(x), "Range(1, 10)")

# Test external generic method for testthat::testthat_print()
expect_equal(testthat::capture_output(x, print = TRUE), "Range(1, 10)")
})
22 changes: 22 additions & 0 deletions tests/testthat/_snaps/S7.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# S7 coverage is reported

Code
cov[, c("functions", "first_line", "last_line", "value")]
Output
functions first_line last_line value
1 Range@properties$length$getter 9 9 1
2 Range@properties$length$setter 11 11 1
3 Range@properties$length$setter 12 12 1
4 Range 17 17 2
5 Range@validator 20 20 5
6 Range@validator 21 21 0
7 Range@validator 22 22 5
8 Range@validator 23 23 0
9 Range@validator 24 24 5
10 Range@validator 25 25 1
11 method(inside, TestS7::Range) 34 34 1
12 method(base::format, TestS7::Range) 43 43 2
13 method(testthat::testthat_print, TestS7::Range) 48 48 1
14 method(testthat::testthat_print, TestS7::Range) 49 49 1
15 .onLoad 53 53 0

4 changes: 3 additions & 1 deletion tests/testthat/test-S7.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
test_that("S7 coverage is reported", {
local_edition(3)
cov <- as.data.frame(package_coverage(test_path("TestS7")))

expect_equal(cov$value, c(1, 1, 1, 1, 4, 0, 4, 0, 4, 1, 1))
expect_equal(cov$value, c(1, 1, 1, 2, 5, 0, 5, 0, 5, 1, 1, 2, 1, 1, 0))
expect_snapshot(cov[, c("functions", "first_line", "last_line", "value")])
})
Loading