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

refactor to remove anonymous functions from S4 method definitions #182

Open
mbjones opened this issue Apr 7, 2017 · 2 comments
Open

refactor to remove anonymous functions from S4 method definitions #182

mbjones opened this issue Apr 7, 2017 · 2 comments
Milestone

Comments

@mbjones
Copy link
Member

mbjones commented Apr 7, 2017

The current dataone implementation creates S4 methods as anonymous functions. This has some side effects, including that 1) stack traces use ".local" as the function name, which isn't very informative, and 2) programming clients can't use method lookups to do autocomplete. We should consider eliminating the use of anonymous functions, replacing them with real functions. Our current approach is like this:

setMethod("foo", signature("character"), function(x) {
    # do something with x
    print(x)
})

which might be replaced with:

foo_ <- function(x) {
    # do something with x
    print(x)
}
setMethod("foo", signature("character"), foo_)

I'm not sure if this approach helps much with autocomplete. It also creates a whole bunch of functions that we don't actually want exported because they shadow the methods.

Also, in the latter approach, I am not sure where the roxygen docs would go, on the function def or on the setMethod() call. Probably before the function, but I'm not sure.

@amoeba, @cboettig, and @gothub: what are your thoughts on what the best course of action would be?

@gothub gothub added this to the 2.2.0 milestone Jun 29, 2017
@gothub gothub modified the milestones: 2.2.0, 2.3.0 Sep 1, 2020
@gothub
Copy link
Collaborator

gothub commented Nov 21, 2020

Related effort ropensci/datapack#130

@mbjones
Copy link
Member Author

mbjones commented Nov 21, 2020

Note that I tested it out in ropensci/datapack#130 and 1) the private method is not exported and so isn't a problem, and 2) roxygen is perfectly happy with the docs on the setMethod calls, so we can leave that as is.

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

No branches or pull requests

2 participants