zoonproject / zoon

The zoon R package
Other
61 stars 13 forks source link

zoon calls in a function fail #391

Open AugustT opened 7 years ago

AugustT commented 7 years ago

If you place a call to workflow in a function it appears not to work as zoon expects the variables to be in the global environment. I haven't dug around too much but this is a bit of an issue as it means that parallelisation becomes a bit of a headache.

work1 <- workflow(occurrence = UKAnophelesPlumbeus,
                  covariate = UKAir,
                  process = Background(n = 70),
                  model = LogisticRegression,
                  output = SameTimePlaceMap)

test <- function(x){

  work1 <- workflow(occurrence = UKAnophelesPlumbeus,
                    covariate = UKAir,
                    process = Background(n = x),
                    model = LogisticRegression,
                    output = SameTimePlaceMap)

}

# fails
test(70)

# This is a hacky fix which works
test2 <- function(x){

  x <<- x

  work <- workflow(occurrence = UKAnophelesPlumbeus,
                    covariate = UKAir,
                    process = Background(n = x),
                    model = LogisticRegression,
                    output = SameTimePlaceMap)

}

test2(70)
goldingn commented 7 years ago

Looks like this is just a lexical scoping issue. The line:

e <- environment() 

in workflow.R defines the environment where the module-running expressions are evaluated. It sets the workflow function itself as the environment. x isn't defined in that environment, so it looks in its parent environment, recursively until it finds x.

But the parent environment of workflow() isn't the calling environment (test()), which is what we want. It's the zoon package namespace, so it keeps looking up the environment inheritance until it hits the global environment.

So I think we just need to change that line to look in the environment from which workflow() is called. I think this should do it:

e <- parent.frame()

though I'm away from my laptop so can't check now.

test() may also need to have force(x) at the top, to avoid lazy evaluation and make sure the object is defined there. Though I'm not sure about that.

goldingn commented 7 years ago

Oh, but it needs to look in workflow() to find the modules, which are loaded there. Hrmmm.

One solution would be to create a new environment, e that inherits from the calling environment, and to load the modules there (using with() or local() or something).

I'll think on this for a bit and see if there is a more elegant solution.

goldingn commented 7 years ago

this is fixed on goldingn/zoon@fix_scoping