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

Revamp method calling #131

Closed
wants to merge 13 commits into from
Closed

Revamp method calling #131

wants to merge 13 commits into from

Conversation

hadley
Copy link
Member

@hadley hadley commented Jan 11, 2022

Builds on #129. Fixes #85. Fixes #88.

Now all non-dispatched arguments are passed through to the method as is. ... no longer has special handling, except as it pertains to method compatibility
@hadley hadley changed the title Method call2 Revamp method calling Jan 11, 2022
@hadley hadley changed the base branch from master to dispatch-args January 11, 2022 19:15
@hadley hadley marked this pull request as ready for review January 17, 2022 21:50
@hadley hadley changed the base branch from dispatch-args to master January 17, 2022 21:56
@hadley hadley changed the base branch from master to dispatch-args January 18, 2022 04:09
R/generic.R Show resolved Hide resolved

if (is.null(fun)) {
args <- setNames(lapply(dispatch_args, function(i) quote(expr = )), dispatch_args)
args <- c(dispatch_args, "...")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is documented? I'm also not entirely sure if it is right or not. I guess to opt out of this, you provide an explicit fun

R/generic.R Outdated Show resolved Hide resolved
R/method.R Outdated
Comment on lines 157 to 164
if (names(generic_formals[i]) == "...") {
# Method doesn't have to have ... even if generic does
next
}

if (!identical(generic_formals[i], method_formals[i])) {
stop(sprintf("`method` must be consistent with <R7_generic> %s.\n- Argument %i in generic %s\n- Argument %i in method %s", generic@name, i, arg_to_string(generic_formals[i]), i, arg_to_string(method_formals[i])), call. = FALSE)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, is this right? what about generic <- function(x, ... y = 1) with method <- function(x, y = 1)? generic_formals would be length 3 and method_formals would be length 2 then right? maybe you need to do generic_formals <- generic_formals[-i] before next

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe a test to ensure that a method can't add dots if the generic doesn't have them

src/dispatch.c Show resolved Hide resolved
src/dispatch.c Outdated
// Lookup the promise for that argument in the environment
SEXP name = Rf_install(CHAR(STRING_ELT(gen_signature_args, i)));
// Find its name and look up its value (a promise)
SEXP name = TAG(Rf_nthcdr(FORMALS(generic), i));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a more efficient way to loop over this FORMALS(generic) pairlist?

src/dispatch.c Outdated Show resolved Hide resolved
src/dispatch.c Outdated Show resolved Hide resolved
src/dispatch.c Outdated Show resolved Hide resolved
@hadley hadley deleted the branch dispatch-args January 26, 2022 18:29
@hadley hadley closed this Jan 26, 2022
@hadley hadley mentioned this pull request Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants