-
Notifications
You must be signed in to change notification settings - Fork 47
Implement external classes #660
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
Open
hadley
wants to merge
28
commits into
main
Choose a base branch
from
external-class
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
9746ecf
Implement external class helper
hadley 9d3a0db
Store full class name
hadley 07e474f
Refactor dep helpers
hadley 9a8806a
Clarify optional/required resolution
hadley 9cbe334
Polish docs
hadley 0cecf0c
Update snapshot
hadley 2e4c50f
Refactoring to clarify intent
hadley 8ae6e5a
Test + vignette tidyup
hadley a156420
Merge branch 'main' into external-class
hadley 1cd4664
Polishing
hadley 45a0d3b
Polish errors
hadley c665edc
Style
hadley 7dd3dc4
Polish vignette
hadley 5234fb7
Merge commit '538ba8b5a0a4d040c821a2698853edb323d63c4a'
hadley ae57980
Use `:=`
hadley fa99764
test: cover external class method edge cases
t-kalinowski def0f13
Fix external class method registration edge cases
t-kalinowski bfb4db5
`air format .`
t-kalinowski 5a2d37f
Add regressions for external class edge cases
t-kalinowski 9e83fd7
Fix external class resolution and deferred method cleanup
t-kalinowski 3915f64
Merge commit 'a4d4f4950fd0554ef78ad005f55aaffacae42e4e'
hadley a6922ee
Reformat
hadley f747a04
This PR does not handle subclassing
hadley 96574d5
Drop class_extends_external helper
hadley 16ca6a2
Use `local_package()` in tests
hadley a831c17
Add comment
hadley 111f82f
Revert roxygen2 changes
hadley 5fdfc29
Refactor hooks logic
hadley File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,209 @@ | ||
| #' Classes in other packages | ||
| #' | ||
| #' @description | ||
| #' An external class is a lightweight placeholder for an S7 class defined in | ||
| #' another package (or in your own package and needed before it's fully | ||
| #' defined). It carries only the package and class name, and is resolved to | ||
| #' the real S7 class when needed. | ||
| #' | ||
| #' External classes are useful in two situations: | ||
| #' | ||
| #' * To register a method for a generic in your package, dispatching on a class | ||
| #' from a soft dependency. The method will be registered when `pkg` is loaded | ||
| #' (using the same machinery as [new_external_generic()]). | ||
| #' | ||
| #' ```R | ||
| #' SomeClass <- new_external_class("pkg", "SomeClass") | ||
| #' method(my_generic, SomeClass) <- ... | ||
| #' ``` | ||
| #' | ||
| #' * To refer to a class that hasn't been defined yet, such as a | ||
| #' self-referential or mutually recursive class. | ||
| #' | ||
| #' ```R | ||
| #' tree_stub <- new_external_class("mypkg", "tree") | ||
| #' new_class("tree", properties = list(child = NULL | tree_stub)) | ||
| #' ``` | ||
| #' | ||
| #' Make sure to call [S7_on_load()] in your package's `.onLoad()` so that | ||
| #' deferred method registrations fire when the relevant package is loaded. | ||
| #' | ||
| #' External classes can not currently be used as parents in [new_class()]. | ||
| #' We hope to relax that restriction in the near future. | ||
| #' | ||
| #' @param package Package the class is defined in. | ||
| #' @param name Name of the class, as a string. | ||
| #' @inheritParams new_external_generic version | ||
| #' @returns An S7 external class, i.e. a list with S3 class `S7_external_class`. | ||
| #' @export | ||
| #' @examples | ||
| #' # Refer to a class in another package without taking a hard dependency: | ||
| #' Tibble <- new_external_class("tibble", "tbl_df") | ||
| #' Tibble | ||
| #' | ||
| #' # Self-referential class: the `child` property can be another `tree`, | ||
| #' # or `NULL` to terminate the chain. | ||
| #' tree_stub <- new_external_class("mypkg", "tree") | ||
| #' tree <- new_class( | ||
| #' name = "tree", | ||
| #' package = "mypkg", | ||
| #' properties = list(child = NULL | tree_stub) | ||
| #' ) | ||
| new_external_class <- function(package, name, version = NULL) { | ||
| if (!is_string(package)) { | ||
| stop2("`package` must be a string.") | ||
| } | ||
| if (!is_string(name)) { | ||
| stop2("`name` must be a string.") | ||
| } | ||
|
|
||
| out <- list( | ||
| package = package, | ||
| name = name, | ||
| class_name = paste0(package, "::", name), | ||
| version = version | ||
| ) | ||
| class(out) <- "S7_external_class" | ||
| out | ||
| } | ||
|
|
||
| is_external_class <- function(x) { | ||
| inherits(x, "S7_external_class") | ||
| } | ||
|
|
||
| class_has_external_class <- function(x) { | ||
| if (is_external_class(x)) { | ||
| TRUE | ||
| } else if (is_union(x)) { | ||
| any(vlapply(x$classes, class_has_external_class)) | ||
| } else { | ||
| FALSE | ||
| } | ||
| } | ||
|
|
||
| signature_has_external_class <- function(signature) { | ||
| any(vlapply(signature, class_has_external_class)) | ||
| } | ||
|
|
||
| class_external_deps <- function(x) { | ||
| if (is_external_class(x)) { | ||
| list(x) | ||
| } else if (is_union(x)) { | ||
| flatten_external_deps(lapply(x$classes, class_external_deps)) | ||
| } else { | ||
| list() | ||
| } | ||
| } | ||
|
|
||
| signature_external_deps <- function(signature) { | ||
| flatten_external_deps(lapply(signature, class_external_deps)) | ||
| } | ||
|
|
||
| flatten_external_deps <- function(x) { | ||
| unlist(x, recursive = FALSE, use.names = FALSE) | ||
| } | ||
|
|
||
| #' @export | ||
| print.S7_external_class <- function(x, ...) { | ||
| cat( | ||
| "<S7_external_class> ", | ||
| x$class_name, | ||
| if (!is.null(x$version)) paste0(" (>= ", x$version, ")"), | ||
| "\n", | ||
| sep = "" | ||
| ) | ||
| invisible(x) | ||
| } | ||
|
|
||
| dep_available <- function(dep) { | ||
| isNamespaceLoaded(dep$package) && | ||
| (is.null(dep$version) || getNamespaceVersion(dep$package) >= dep$version) | ||
| } | ||
|
|
||
| # Make it mockable | ||
| getNamespaceVersion <- NULL | ||
|
|
||
| resolve_signature <- function(signature) { | ||
| for (i in seq_along(signature)) { | ||
| signature[[i]] <- resolve_class_req(signature[[i]]) | ||
| } | ||
| signature | ||
| } | ||
|
|
||
| resolve_class_req <- function(x) { | ||
| if (is_external_class(x)) { | ||
| resolve_external_class_req(x) | ||
| } else if (is_union(x)) { | ||
| do.call(new_union, lapply(x$classes, resolve_class_req)) | ||
| } else { | ||
| x | ||
| } | ||
| } | ||
|
|
||
| find_external_class <- function(x) { | ||
| ns <- asNamespace(x$package) | ||
| if (exists(x$name, envir = ns, inherits = FALSE)) { | ||
| obj <- get(x$name, envir = ns, inherits = FALSE) | ||
| if (is_external_class_match(obj, x)) { | ||
| return(obj) | ||
| } | ||
| } | ||
|
|
||
| # Also consider cases where the constructor isn't named the same as the class | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure that this is a case that we want to support? Why do we want to encourage people to have a mismatch between constructor and class name? |
||
| for (name in ls(ns, all.names = TRUE)) { | ||
| obj <- get(name, envir = ns, inherits = FALSE) | ||
| if (is_external_class_match(obj, x)) { | ||
| return(obj) | ||
| } | ||
| } | ||
|
|
||
| NULL | ||
| } | ||
|
|
||
| is_external_class_match <- function(obj, x) { | ||
| is_class(obj) && | ||
| (identical(S7_class_name(obj), x$class_name) || | ||
| (identical(obj@name, x$name) && | ||
| (is.null(obj@package) || identical(obj@package, x$package)))) | ||
| } | ||
|
|
||
| # Required resolution: errors if the external class can't be resolved (e.g. | ||
| # its package isn't loaded). Used wherever we need the real class: registering | ||
| # or looking up methods, checking property overrides in a subclass, and | ||
| # constructing or validating an instance. | ||
| resolve_external_class_req <- function(x) { | ||
| prefix <- sprintf("Can't find external class <%s>:\n", x$class_name) | ||
| if (!requireNamespace(x$package, quietly = TRUE)) { | ||
| stop2( | ||
| paste0(prefix, sprintf("* Package '%s' is not installed.", x$package)), | ||
| call = NULL | ||
| ) | ||
| } | ||
|
|
||
| if (!is.null(x$version) && getNamespaceVersion(x$package) < x$version) { | ||
| stop2( | ||
| paste0( | ||
| prefix, | ||
| sprintf( | ||
| "* Package '%s' needs version %s, but only %s is available.", | ||
| x$package, | ||
| x$version, | ||
| getNamespaceVersion(x$package) | ||
| ) | ||
| ), | ||
| call = NULL | ||
| ) | ||
| } | ||
|
|
||
| class <- find_external_class(x) | ||
| if (is.null(class)) { | ||
| stop2( | ||
| paste0( | ||
| prefix, | ||
| sprintf("* Packages '%s' doesn't contain '%s'.", x$package, x$name) | ||
| ), | ||
| call = NULL | ||
| ) | ||
| } | ||
| class | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made this restriction more clear. Next step is to resolve #609, and #317.