Skip to content

Fix base referencing.#50

Open
robertzk wants to merge 1 commit intomasterfrom
fix_base_referencing
Open

Fix base referencing.#50
robertzk wants to merge 1 commit intomasterfrom
fix_base_referencing

Conversation

@robertzk
Copy link
Copy Markdown
Member

@robertzk robertzk commented Jun 22, 2017

@abelcastilloavant @peterhurford @kirillseva This little snaggler came up when I had a global variable match <- function(...) { foo }, since it gets used in the body of column transformations, which passes through globalenv() when searching parent environments.

I think this may be too slow for column transformation, however, given that under the hood we are calling

`::`(quote(base), quote(foo))

repeatedly. Potentially a better alternative is to do the silly thing and copy those aliases match <- base::match into the package and shadow them directly for a boost. I'll do some benchmarks tomorrow.

Also need to change multi_column_transformation.

## checks for the sake of performance and use straight-up list subsetting
## (which will use underlying C code).
class(data) <- "list"
class(data) <- "list" # If you've overwritten `class<-` globally then too bad.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Or isTRUE, assign, (, etc. lol

## has been accidentally overwritten (e.g., if someone makes a
## global `substitute` variable that points to a function).
##
## Yes, people are silly, and this does sometimes happen!
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is generally a good idea.

@robertzk
Copy link
Copy Markdown
Member Author

Some failing tests. I'll address later this week.

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.

3 participants