wahani / modules

Modules in R
https://cran.r-project.org/package=modules
Other
81 stars 3 forks source link

Extra brackets required when used with %>% operator #7

Closed jeremyhoughton closed 7 years ago

jeremyhoughton commented 7 years ago

When a function is put into a module and then that function is called outside that module using the pipe forward operator %>%, an extra set of brackets () are required in the function call. This is not the case when the exact same function lives outside the module.

In an ideal world the syntax of the function call at the call site should not depend on whether the function lives in a module or not (apart from the qualifying module name of course).

Please see a small code snippet below reproducing this issue.

install.packages("modules")

library(modules)

medianWrapper <- function (data) {
  median(data)
}

Math <- module({

  import("stats", "median")

  medianWrapperMod <- function (data) {
    median(data)
  }

})

# Setup some testing data
dummyData <- c(1:10)

# Call medianWrapper function outside module
dummyData %>%
  medianWrapper

# Call medianWrapperMod function inside module
# Throws the following error: "Error in .$Math : 3 arguments passed to '$' which requires 2"
dummyData %>%
  Math$medianWrapperMod

# Call medianWrapperMod function inside module with extra brackets () works
# however the extra () makes the calling syntax different to the above which can be confusing
dummyData %>%
  Math$medianWrapperMod()
wahani commented 7 years ago

Hi, thanks for taking the time to report this issue!

I agree that this behavior is unexpected because that way it seems that functions in a module are somehow different from functions outside of a module. However I think the behavior we observe is due to what %>% expects when it parses the $ sign in a right hand side expression. Also this means that we cannot do much about it in the modules package. Even reporting it as an issue in magrittr may not work because there it might be some feature(?).

I compiled some examples to illustrate that it is not about using a module in particular but any form of $-calls. I assume it will also not work for R6 classes, reference classes (see below), or any of those packages but I haven't checked it:

# works:
dummyData %>%
  (Math$medianWrapperMod)

# also works:
dummyData %>%
  (Math$medianWrapperMod)()

# Does not work:
MathList <- list()
MathList$medianWrapperMod <- function(data) {
  stats::median(data)
}
dummyData %>%
  MathList$medianWrapperMod

# Does also not work
setRefClass("MathClass", methods = list(
  medianWrapperMod = function(data) {
    stats::median(data)
  }
))

mathInstance <- new("MathClass")
dummyData %>%
  mathInstance$medianWrapperMod
wahani commented 7 years ago

This will work:

with(Math, {
  dummyData %>% medianWrapperMod
})

but I don't know if you think it is better than placing the braces.

jeremyhoughton commented 7 years ago

Thanks so much for the quick response. As you say this is definitely an issue with the $ sign and the magrittr package. I'm not too sure why this would be a feature? I will raise as a separate issue in the magrittr repo though.

Both the With clause and the braces method are very good work arounds for this issue.

wahani commented 7 years ago

Happy to hear it. Reporting this in magrittr is a probably good idea!