Config branch... #402

ldecicco-USGS · 2016-08-15T17:06:03Z

I had been using the dev.off to fix the bleeding par, but that had an unintended consequence when kniting. This PR instead goes back to the original scheme where we'd save the original par in the gsconfig, then reset all of those pars if someone changes the config (either with loadConfig or in the call to the gsplot object). This somewhat alters all the figures because now the background tends to be translucent instead of white. I actually think that's better, but would like to hear more discussion.

Merge branch 'master' of github.com:USGS-R/gsplot into configBranch # Conflicts: # R/config.R # inst/doc/gsplotIntro.html

Merge branch 'master' of github.com:ldecicco-USGS/gsplot into configBranch # Conflicts: # inst/doc/gsplotIntro.html

coveralls · 2016-08-15T17:14:05Z

Coverage decreased (-0.4%) to 79.102% when pulling c287996 on ldecicco-USGS:configBranch into bc8f472 on USGS-R:master.

lindsayplatt · 2016-08-15T21:45:25Z

R/config.R

@@ -1,4 +1,5 @@
 gsconfig <- new.env(parent = emptyenv())
+gsconfig$orignial.par <- par(no.readonly = TRUE)


@ldecicco-USGS "original"

yeah, learned how to wipe out a branch and this is what happens

coveralls · 2016-08-16T02:51:21Z

Coverage decreased (-0.5%) to 79.034% when pulling 272cede on ldecicco-USGS:configBranch into bc8f472 on USGS-R:master.

coveralls · 2016-08-16T03:21:29Z

Coverage increased (+0.1%) to 79.638% when pulling bbfc10f on ldecicco-USGS:configBranch into bc8f472 on USGS-R:master.

lindsayplatt · 2016-08-19T16:56:33Z

@ldecicco-USGS I'm on your branch (and built/reloaded), but this is still causing an issue. It's like the way legend handles par is messing it up because it looks ok when I exclude legend.name. However, not sure why the axis() call is being ignored for changing side.2$lim.

g1 <- gsplot() %>% 
  lines(1:9, seq(1,3, by=0.25), 
        legend.name="second line") %>% 

  # formatting y axes
  axis(side=2, at=1:10) 

layout(matrix(1:2, 1, 2))
g1
g1

ldecicco-USGS · 2016-08-19T17:03:44Z

I did not test out my branch at all with layout...I'm not sure what happens with par there.

I totally agree that legend is mucking up nice par behaviors. I've noticed this in the past too. It's the only function that directly changes par, and then unchanges it back at the end. I'm guessing that's the root of the problem.

I'm guessing what you are seeing there is something I've been concerned about for a little while now. Because we are changing par in the building up of the gsplot object....it's possible to get 2 different printed outputs using the same object if par gets changed from one print to the next. I'm guessing that's what's going on here. I'd have to explore what layout does with par to know what's going on here and how we'd want to deal with it in the longer term.

lindsayplatt · 2016-08-19T17:05:00Z

It happens when you just print the plot twice in a row as well, layout was just easier to show them side by side

lindsayplatt · 2016-08-19T17:07:10Z

It only changes par when draw_legend is called, which doesn't happen here because I never have the legend drawn. I only added legend.name, but never used legend(). So I think something else is at play here

lindsayplatt · 2016-08-19T17:08:32Z

Actually, calling legend() just made that issue stop...what is happening?!

g1 <- gsplot() %>% 
  lines(1:9, seq(1,3, by=0.25), 
        legend.name="second line") %>% 

  # formatting y axes
  axis(side=2, at=1:10) %>% legend()

layout(matrix(1:2, 1, 2))
g1
g1

ldecicco-USGS · 2016-08-19T17:09:14Z

I'm just saying:

I think we need to re-think how we deal with draw_legend in general I know this is not the case that you are talking about right here and now
This particular case is different, and I'm looking into it (personally, on my own time at home tonight)

ldecicco-USGS · 2016-08-19T17:12:32Z

Here's how I'd start doing some troubleshooting for par things:

g1 <- gsplot() %>% 
  lines(1:9, seq(1,3, by=0.25), 
        legend.name="second line") %>% 
  axis(side=2, at=1:10) 
x <- par(no.readonly = TRUE)
g1
y <- par(no.readonly = TRUE)
all.equal(x, y)
 [1] "Component “mgp”: Mean relative difference: 0.55"      
 [2] "Component “pin”: Mean relative difference: 0.1922864" 
 [3] "Component “plt”: Mean relative difference: 0.1891681" 
 [4] "Component “tcl”: Mean relative difference: 1.6"       
 [5] "Component “usr”: Mean relative difference: 6"         
 [6] "Component “xaxp”: Mean relative difference: 1.833333" 
 [7] "Component “xaxs”: 1 string mismatch"                  
 [8] "Component “xpd”: 1 element mismatch"                  
 [9] "Component “yaxp”: Mean relative difference: 0.6666667"
[10] "Component “yaxs”: 1 string mismatch"

ldecicco-USGS · 2016-08-19T17:15:21Z

The difference between the 2 graphs in par seems to be xpd...

g1 <- gsplot() %>% 
  lines(1:9, seq(1,3, by=0.25), 
        legend.name="second line") %>% 
  axis(side=2, at=1:10) %>% legend()
x <- par(no.readonly = TRUE)
g1
y <- par(no.readonly = TRUE)
all.equal(x, y)
[1] "Component “mgp”: Mean relative difference: 0.55"      
[2] "Component “pin”: Mean relative difference: 0.1922864" 
[3] "Component “plt”: Mean relative difference: 0.1891681" 
[4] "Component “tcl”: Mean relative difference: 1.6"       
[5] "Component “usr”: Mean relative difference: 6"         
[6] "Component “xaxp”: Mean relative difference: 1.833333" 
[7] "Component “xaxs”: 1 string mismatch"                  
[8] "Component “yaxp”: Mean relative difference: 0.6666667"
[9] "Component “yaxs”: 1 string mismatch"

lindsayplatt · 2016-08-19T17:21:41Z

Oh and having xpd be different would explain all of that. Adding this fixes the issue, so now I can figure out where that's being changed.

g1 <- gsplot(xpd=FALSE) %>% 
  lines(1:9, seq(1,3, by=0.25), 
        legend.name="second line") %>% 

  # formatting y axes
  axis(side=2, at=1:10) 

layout(matrix(1:2, 1, 2))
g1
g1

par(xpd=TRUE) was in the wrong place

lindsayplatt · 2016-08-19T17:51:01Z

should we up to 0.7.1 when this merges??

ldecicco-USGS · 2016-08-19T17:52:05Z

Yeah

coveralls · 2016-08-19T17:52:51Z

Coverage increased (+0.1%) to 79.638% when pulling 20c9a11 on ldecicco-USGS:configBranch into bc8f472 on USGS-R:master.

lindsayplatt · 2016-08-22T13:54:26Z

I tested this with repgen and the only thing I noticed that was different was a legend was being cutoff. I'm trying to recreate a simple example.

lindsayplatt · 2016-08-25T15:46:22Z

Something with this branch seems to be ignoring par.

par(mar=c(5,4,10,2))

g1 <- gsplot() %>% 
  points(1:10, 1:10, pch=20, legend.name="first points") %>% 
  lines(4:1, 4:1, legend.name="first line") %>% 
  points(c(3,7,4), c(9,3,6), pch=20, col="black", legend.name="second points") %>% 
  legend(location="above")
g1

v0.7.0

this branch

ldecicco-USGS · 2016-08-25T16:39:55Z

I can fix that...maybe by the end of the day...silly mistake

lindsayplatt · 2016-08-25T17:37:42Z

Oh ok cool! Think that might also fix #405 ??

ldecicco-USGS · 2016-09-01T04:20:22Z

Forgot about this...will try to do soon

lindsayplatt · 2016-09-01T12:44:30Z

I was thinking we could take a look tomorrow at the sprint planning meeting as well

jordansread · 2016-09-02T09:25:54Z

R/add_to_view.R

@@ -70,14 +71,14 @@ add_to_view <- function(object, call.args, side, where){
 #' gsplot:::filter_arguments('points', x=1:5, y=1:5, xlim=c(0,10), ylim=c(0,10), 
 #'                callouts(labels=c(rep(NA, 4), "oh")))$extracted.args
 #' @keywords internal
-filter_arguments <- function(fun.name, ...){
+filter_arguments <- function(fun.name, ..., custom.config = FALSE){


was thinking that default would be custom.config = NULL since that is what you would get if
custom.config = object[["global"]][["config"]][["config.file"]] didn't exist. Maybe I missed it, but where do you test for !custom.config or where is that used?

lindsayplatt · 2016-10-19T18:32:54Z

@ldecicco-USGS do you remember what we concluded about this? Is there more work to do?

I see that it potentially fixes #405, #392, #130

lindsayplatt · 2016-10-24T18:54:22Z

par issues: #254

pass config a list or a filename

…onfigBranch

ldecicco-USGS · 2016-11-07T19:05:56Z

I'm just going to start over

ldecicco-USGS added 10 commits August 5, 2016 10:39

trying out config stuff

172875f

upstream commit

05e8d9f

Merge branch 'master' of github.com:USGS-R/gsplot into configBranch # Conflicts: # R/config.R # inst/doc/gsplotIntro.html

Initial work on config.file

6b7fe06

temp config

95278a8

knit?

7ce878c

rebuild vignette

a473d8c

playing with knitr

caa3abd

Fixing bleeding par broke knitr.

a71d121

Chunk's back.

754800d

RAther than dev.off, re-define par.

c287996

Merge branch 'master' of github.com:ldecicco-USGS/gsplot into configBranch # Conflicts: # inst/doc/gsplotIntro.html

lindsayplatt reviewed Aug 15, 2016
View reviewed changes

responses to review

272cede

Add some tests

bbfc10f

ldecicco-USGS changed the title ~~Config branch...somewhat WIP~~ Config branch... Aug 17, 2016

par(xpd=TRUE) was in the wrong place

51123dd

Merge pull request #1 from lindsaycarr/ldecicco-USGS-configBranch

20c9a11

par(xpd=TRUE) was in the wrong place

lindsayplatt mentioned this pull request Aug 24, 2016

upgrade to gsplot v0.7.0 USGS-R/lakeReport#29

Open

jordansread reviewed Sep 2, 2016
View reviewed changes

This was referenced Oct 19, 2016

legend squished with axes #405

Open

Multiple defaults for one parameter #204

Open

MVP for soft release #407

Open

ldecicco-USGS added 4 commits November 3, 2016 14:43

Some updates

cde8a6a

Merge branch 'master' of github.com:USGS-R/gsplot into configBranch

a2cb09b

Merge branch 'configBranch' of github.com:ldecicco-USGS/gsplot into c…

e2a29c1

…onfigBranch

Merge branch 'master' of github.com:USGS-R/gsplot into configBranch

c74b2a8

ldecicco-USGS closed this Nov 7, 2016

ldecicco-USGS deleted the configBranch branch February 10, 2017 15:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Config branch... #402

Config branch... #402

ldecicco-USGS commented Aug 15, 2016

coveralls commented Aug 15, 2016

lindsayplatt Aug 15, 2016

ldecicco-USGS Aug 15, 2016

ldecicco-USGS Aug 15, 2016

coveralls commented Aug 16, 2016

coveralls commented Aug 16, 2016

lindsayplatt commented Aug 19, 2016

ldecicco-USGS commented Aug 19, 2016

lindsayplatt commented Aug 19, 2016

lindsayplatt commented Aug 19, 2016

lindsayplatt commented Aug 19, 2016

ldecicco-USGS commented Aug 19, 2016

ldecicco-USGS commented Aug 19, 2016

ldecicco-USGS commented Aug 19, 2016

lindsayplatt commented Aug 19, 2016

lindsayplatt commented Aug 19, 2016

ldecicco-USGS commented Aug 19, 2016

coveralls commented Aug 19, 2016

lindsayplatt commented Aug 22, 2016

lindsayplatt commented Aug 25, 2016

ldecicco-USGS commented Aug 25, 2016

lindsayplatt commented Aug 25, 2016

ldecicco-USGS commented Sep 1, 2016

lindsayplatt commented Sep 1, 2016

jordansread Sep 2, 2016

lindsayplatt commented Oct 19, 2016

lindsayplatt commented Oct 24, 2016 •

edited

Loading

ldecicco-USGS commented Nov 7, 2016

		@@ -1,4 +1,5 @@
		gsconfig <- new.env(parent = emptyenv())
		gsconfig$orignial.par <- par(no.readonly = TRUE)

Config branch... #402

Config branch... #402

Conversation

ldecicco-USGS commented Aug 15, 2016

coveralls commented Aug 15, 2016

lindsayplatt Aug 15, 2016

Choose a reason for hiding this comment

ldecicco-USGS Aug 15, 2016

Choose a reason for hiding this comment

ldecicco-USGS Aug 15, 2016

Choose a reason for hiding this comment

coveralls commented Aug 16, 2016

coveralls commented Aug 16, 2016

lindsayplatt commented Aug 19, 2016

ldecicco-USGS commented Aug 19, 2016

lindsayplatt commented Aug 19, 2016

lindsayplatt commented Aug 19, 2016

lindsayplatt commented Aug 19, 2016

ldecicco-USGS commented Aug 19, 2016

ldecicco-USGS commented Aug 19, 2016

ldecicco-USGS commented Aug 19, 2016

lindsayplatt commented Aug 19, 2016

lindsayplatt commented Aug 19, 2016

ldecicco-USGS commented Aug 19, 2016

coveralls commented Aug 19, 2016

lindsayplatt commented Aug 22, 2016

lindsayplatt commented Aug 25, 2016

v0.7.0

this branch

ldecicco-USGS commented Aug 25, 2016

lindsayplatt commented Aug 25, 2016

ldecicco-USGS commented Sep 1, 2016

lindsayplatt commented Sep 1, 2016

jordansread Sep 2, 2016

Choose a reason for hiding this comment

lindsayplatt commented Oct 19, 2016

lindsayplatt commented Oct 24, 2016 • edited Loading

ldecicco-USGS commented Nov 7, 2016

lindsayplatt commented Oct 24, 2016 •

edited

Loading