USGS-R / gsplot

plotting foundation for timeseries reporting
Other
6 stars 14 forks source link

If side 1 or 2 isn't defined with data, can't modify axis. #453

Closed ldecicco-USGS closed 7 years ago

ldecicco-USGS commented 7 years ago
gsplot() %>%
     points(1:100, 1:100, log="xy", side=c(3,4)) %>%
     axis(side=c(1), log="") 

image

Trying to figure out a way to not log side 1.

jordansread commented 7 years ago

per #456 , log is being ignored here because it isn't a formal for any of the axis functions that gsplot looks to for arg names. My fix for #456 (WIP) causes your call to warn with

Warning message:
In axis(side = side, at = at, labels = labels, ...) :
  "log" is not a graphical parameter

which I think is right.

Do you want

gsplot() %>%
    points(1:100, 1:100, log="xy", side=c(3,4)) %>%
    view(log='', side=c(1,2))
    axis(side=c(1)) 

That does the same thing but is the way you could modify the log status of 1 and 2 if there were data there.

weirder yet is that this seems to ignore logging by tying the axes together too strongly:

gsplot() %>%
    points(1:100, 1:100, log="xy", side=c(3,4)) %>%
    points(1:10,1:10, log="", side=c(1,2)) %>% 
    axis(side=c(1)) 

image

Do we have tests hitting this code? https://github.com/USGS-R/gsplot/blob/455298e9de2b6dcfda7a9b9232d4c2b9f8727ce7/R/modify_side.R#L31

ldecicco-USGS commented 7 years ago

I think we really need to emphasize the view function in the readme and vignette, since it's so critical and completely unlike base R. I would never think to use the view function, but makes sense once I see it.

lindsayplatt commented 7 years ago

I agree, going to add an issue to add view as documentation in the README and vignettes. See #462

lindsayplatt commented 7 years ago

What's the plan for finishing this one up @ldecicco-USGS @jread-usgs ? The axis issues for axis style args not getting recognized were fixed between #459 and #460. Do we just need to use view here instead and make sure to document that? Or are there code changes that need to take place?

jordansread commented 7 years ago

no, I think this is a bug.

gs <- gsplot() %>%
    points(1:100, 1:100, log="xy", side=c(3,4)) %>%
    points(1:10,1:10, log="", side=c(1,2))

logged(gs)
$side.1
[1] TRUE

$side.2
[1] TRUE

$side.3
[1] TRUE

$side.4
[1] TRUE

sides 1 and 2 shouldn't be logged.

ldecicco-USGS commented 7 years ago

Yeah, I've started this bug fix. I can continue or assign it to someone else if someone's interested. But for now I'm trying to clean it up.

jordansread commented 7 years ago
gs <- gsplot() %>%
     points(1:100, 1:100, log="xy", side=c(3,4)) %>%
     points(1:10,1:10, log="", side=c(1,2))
logged(gs)
$side.1
[1] FALSE

$side.2
[1] FALSE

$side.3
[1] TRUE

$side.4
[1] TRUE

from devtools::install_github('usgs-r/gsplot@v0.7.0')

ldecicco-USGS commented 7 years ago

yeah, but a bunch of the other issues crop back up. I'll have you do the PR review @jread-usgs when it's ready.

jordansread commented 7 years ago

either way, we need a test(s) for this