modify_legend (WIP)

[coveralls]

Coverage decreased (-0.06%) to 70.327% when pulling 39d6204628935df98efab011f92917e94ae9b594 on lindsaycarr:master into fc80a58ae698939973c8bde55f4e49bb985dabed on USGS-R:master.

[lindsayplatt]

• modify_legend should create one legend.args list element (not append multiple to the gsplot object)
• draw_legend currently mushes all of the legend.args together, but this should be handled in modify_legend
• draw_legend should only be calling the graphics legend()

[lindsayplatt]

Currently breaks everything that is not points because modify_legend takes diff arguments than set_legend_args did.

To do tomorrow so that it is no longer broken:

• [x] get the rest of set_legend_args functionality into modify_legend (multiple legend args in one function call, etc)
• [x] figure out how/when to do overall legend stuff (when are configs like location and border set)
• [ ] change set_legend_args to modify_legend in all function calls
• [ ] update tests

[lindsayplatt]

failing on abline and rect calls: fails at modify_legend >> call_arguments >> normal_arguments >> set_args >> function_args >> getFromNamespace

[lindsayplatt]

The following test passes when I replace NA with NAinteger, but that is not a new test. I'm wondering why it did not fail before. New package update maybe?

gs <- gsplot() %>%  
    points(x=1, y=2, legend.name="Points 1", pch=1, col="blue") %>% 
    points(x=3, y=4, legend.name="Points 2", pch=5, col="red") %>%  
    error_bar(3,4, y.high=0.2, y.low=0.3, legend.name="Errors", text.font=2) %>% 
    rect(2,3,2.5,4, col="blue", border="orange", lwd=3, density=15, legend.name="rectangle")  %>% 
    legend(location="bottomright", bg="lightgrey")

    expect_equal(gs$legend$legend.args$lwd[4], NA)

This one is failing somewhere in call_arguments >> normal_arguments >> set_args

gs <- gsplot() %>%
     curve(sin(x), from=-2*pi, to=2*pi, legend.name="sin(x)") %>%
     legend()

Error in eval(expr, envir, enclos) : could not find function "*"

[jordansread]

this is really bizarre - the environment for the lazy_eval is null when you do something else w/ the dots

eg <- function(...){
     print(names(list(...)))
     lazyeval::lazy_eval(lazyeval::lazy_dots(...))
}

eg(from=2 * pi)
[1] "from"
 Error in eval(expr, envir, enclos) : could not find function "*" 

#vs comment out the `print`:
eg <- function(...){
     #print(names(list(...)))
     lazyeval::lazy_eval(lazyeval::lazy_dots(...))
}
eg(from=2 * pi)
$from
[1] 6.283185

not what I would expect to happen.

This is a majorly simplified example of what is happening w/ that test in your PR.

[jordansread]

The lazy eval thing is something I don't understand. But the lazy dots things in curve aren't actually needed so that odd behavior can be avoided.

I think curve.gsplot can be re-written as:

curve.gsplot <- function(object, expr, from=0, to=1, n=101, ..., legend.name=NULL, side=c(1,2)){

  expr <- lazy(expr)

  x <- seq(from, to, length.out=n)
  y <- lazy_eval(expr, data.frame(x=x))
  object <- lines(object, x=x, y=y, ..., legend.name=legend.name, side=side)
  return(object)
}

That fixes the test issue you are running into and deals with the minor issues pointed out in #236

[lindsayplatt]

@jread-usgs added a fix for calling legend() without any legend.names. For curve, should we do something different for the n default? Like keep increment <- (arguments$to-arguments$from)/10000 this?

[lindsayplatt]

Oh, just noticed that n=101 is the actual default. I leave that be.

When n is specified is causes an error:

gs <- gsplot() %>%
    curve(sin(x), from=-2*pi, to=2*pi, n=10, legend.name="sin(x)") %>%
    legend()
gs

Error in function_args(package, name, object, ...) : object 'x' not found 

[jordansread]

Hmm - I can't tell where you modified the curve.R file. Is that in here?

[lindsayplatt]

@jread-usgs just pushed from local

[coveralls]

Coverage increased (+2.3%) to 73.464% when pulling 7069295685c2825b45af9baeb53731cdc7e7c540 on lindsaycarr:master into 67aa258e3c9efe4c8886559415e74932bc666bcc on USGS-R:master.

[jordansread]

@lindsaycarr all good to merge?

[lindsayplatt]

Yay! Just saw this :)

[jordansread]

We'll need to be careful with capturing the dots in proper order to make sure we don't run into that null env issue: https://github.com/hadley/lazyeval/issues/61