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

Add envir_prep to checker function call #175

Merged
merged 4 commits into from
Jul 30, 2018

Conversation

schloerke
Copy link
Collaborator

Fixes #163

Submits a copy of the exercise environment before the submission is rendered. This gives a fresh environment for the checker to user that contains the setup code.

@jjallaire
Copy link
Member

We try not to have so many spurious whitespace changes in a diff (makes it hard to tell what material code changes there are). It would be great if you could remove these.

If you feel strongly about enforcing a whitespace convention I'd suggest just one big commit that fixes all the whitespace (and updates the .Rproj file to have a setting that agrees with the scheme).

@schloerke
Copy link
Collaborator Author

schloerke commented Jul 30, 2018

@jjallaire Thanks for the feedback! I've reduced the changes. (I'll make a separate PR for .Rproj whitespace changes later.)

I dropped compiling within the ./docs folder as it produces many changes. Similar to shiny, I think we can wait until CRAN release.

  • NEWS item
  • test?

@jjallaire
Copy link
Member

Great, thanks!

@jjallaire jjallaire merged commit 673837d into rstudio:master Jul 30, 2018
# Create an new, 'twin' environment with the same objects and same parent.
twin_env <- function(envir, parent = parent.env(envir)) {
new_envir <- new.env(parent = parent)
for (object in ls(envir = envir))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this use all.names=TRUE?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Some thoughs that aren't super important, but they might be slightly useful because the function is exported....

It might be faster with this, though you should probably test.

list2env(as.list.environment(envir, all.names = TRUE, sorted = FALSE))

Some notes:

The sorted=FALSE is because sorting can take a nontrivial amount of time.

system.time(ls(baseenv()))
#>   user  system elapsed 
#>  0.007   0.000   0.006 

system.time(ls(baseenv(), sorted=FALSE))
#>   user  system elapsed 
#>  0.003   0.000   0.003 

The reason it uses an explicit as.list.environment is because if the environment happens to have a class on it, it will error. For example:

e <- new.env()
e$x <- 1
class(e) <- "foo"

as.list(e)
#> Error in as.vector(x, "list") : 
#>   cannot coerce type 'environment' to vector of type 'list'

as.list.environment(e)
#> $x
#> [1] 1



# Create an new, 'twin' environment with the same objects and same parent.
twin_env <- function(envir, parent = parent.env(envir)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change to duplicate_env, and export this.

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