dankelley / oce

R package for oceanographic processing
http://dankelley.github.io/oce/
GNU General Public License v3.0
143 stars 42 forks source link

`plotTS` margin behaviour conflicts with `drawPalette` #317

Closed richardsc closed 11 years ago

richardsc commented 11 years ago

It appears that plotTS() imposes it's own margins regardless of what have previously been set. An example is when using drawPalette to add a colorbar.

library(oce)
data(ctd)
col <- oceColorsJet(100)[rescale(ctd[['pressure']], rlow=1, rhigh=100)]
drawPalette(range(ctd[['pressure']]), col=oceColorsJet(100))
plotTS(ctd, col=col)
dankelley commented 11 years ago

Yes, and even below doesn't do the right thing. I'll look into this. Thanks

library(oce)
data(ctd)
col <- oceColorsJet(100)[rescale(ctd[['pressure']], rlow=1, rhigh=100)]
drawPalette(range(ctd[['pressure']]), col=oceColorsJet(100))
plotTS(ctd, col=col, mar=par('mar'))
dankelley commented 11 years ago

It's not at all clear to me why plotTS() should have mar as an arg, since a person can do

par(mar=MAR)
plot(...)

just about as easily as

plot(..., mar=MAR)

but, looking at the code, I'm a bit scared to just remove the arg, since its use depends also on args inset and add (see ctd.R near line 1802).

dankelley commented 11 years ago

In the develop branch, I just put in a temporary debug=10 ability. It will be removed later when I can think this through.

It gets part of the way to success, but now the isopycnal labels are running into the palette. I'll look more at perhaps 3pm my time.

## TEMPORARY action for debug=10 while working on issue 317.
library(oce)
data(ctd)
col <- oceColorsJet(100)[rescale(ctd[['pressure']], rlow=1, rhigh=100)]
drawPalette(range(ctd[['pressure']]), col=oceColorsJet(100))
plotTS(ctd, col=col, debug=10) # 10 means to avoid setting mar
dankelley commented 11 years ago

Below is a demo of the over-pltting into palette, with above test code

rplots

dankelley commented 11 years ago

The code below gives better results.

## TEMPORARY action for debug=10 while working on issue 317.
library(oce)
data(ctd)
col <- oceColorsJet(100)[rescale(ctd[['pressure']], rlow=1, rhigh=100)]
drawPalette(range(ctd[['pressure']]), col=oceColorsJet(100), mai=c(0, 0.5, 0, 0.5))
plotTS(ctd, col=col, debug=10) # 10 means to avoid setting mar

rplots

dankelley commented 11 years ago

@richardsc -- if the prev seems ok, it will be a temporary workaround. I think a fair bit of testing is needed on inset and add before I alter mar, however.

I don't want to break a lot of code by removing the mar arg (and its default). Maybe you can test some of your code with debug=10 to see?

PS. I could also do this as

plotTS(..., mar=NULL)

which, come to think of it, is more sensible than using debug=10, but there's not much sense in doing that if I'm going to remove the mar in a day or two, anyway.

richardsc commented 11 years ago

The fix with the debug=10 option looks pretty good to me. I don't fully understand the reasons behind why the mar argument is specified like that -- probably just legacy coding from the early days of "oce"?

As for the rho label and palette overplotting, perhaps the default for rotateRhoLabels should be TRUE, which saves a bit of space between the axis and the palette. I guess your fix above is just to add a bit more to the palette mar/mai before calling plotTS() which is an ok solution too.

dankelley commented 11 years ago

In develop, I have switched it so that mar=NULL will avoid setting margins. @richardsc -- I'd like to release to CRAN in a few days, with the recent CNV bug fix, so I hope this quick change in args (from the temporary debug=10 method) is okay for you.

richardsc commented 11 years ago

Looks good to me!

From personal preference I'd set rotateRhoLabels=TRUE by default, but I'll leave that as your call.