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

findGlobals() incorrectly detects 'x' as a global in subset(df, x < 3) #1

Closed
HenrikBengtsson opened this issue May 19, 2015 · 2 comments

Comments

@HenrikBengtsson
Copy link
Collaborator

Problem

findGlobals() incorrectly detects x as a global in subset(df, x < 3), e.g.

> library(globals)
> df <- data.frame(x=1:5, y=(1:5)^2)
> findGlobals(substitute({ subset(df, x < 3) }))
[1] "{"      "<"      "df"     "subset" "x"
}

This is a problem with codetools::findGlobals() that globals uses internally.

Workaround

The workaround is to not use subset(), e.g.

> library(globals)
> df <- data.frame(x=1:5, y=(1:5)^2)
> findGlobals(substitute({ keep <- (df$x < 3); df[keep,] }))
[1] "$"  "("  "["  "{"  "<"  "<-" "df"

or to create fake local variables, e.g.

> library(globals)
> df <- data.frame(x=1:5, y=(1:5)^2)
> findGlobals(substitute({ x <- NULL; subset(df, x < 3) }))
[1] "{"      "<"      "<-"     "df"     "subset"
}

Possible approach

Maybe it is possible to add some type of metadata to subset() that give enough hints for findGlobals() to conclude that x could be part of the df object and/or the subset argument is passed without being evaluated. For instance,

argument_hint(subset, "subset", as="substitute")

such that findGlobals() will known that argument subset of subset() is passed as a "substituted" expression. This could then give findGlobals() the option to ignore whatever is passed to argument subset. With even more information, we might also be able to guide findGlobals() to - at least at run time - validate that x is available from either first argument (df) or the calling environment, e.g.

argument_hint(subset, "subset", where=list(argument=1L, "parent.frame"))

Now, could this be inferred from code inspection of subset() itself?

@HenrikBengtsson
Copy link
Collaborator Author

The codetools package uses so called usage handlers internally to handle this for common cases, e.g. base::library(tools), cf. Issue #12.

@HenrikBengtsson
Copy link
Collaborator Author

This works for futures since future (>= 1.0.0) because it now takes an optimistic approach to globals, i.e. it accepts false-positive globals (including non-existing ones as here).

Closing this since the globals package mostly exists such that the future package works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant