dankelley / oce

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

plot,ctd-method should show some isobaths #1052

Closed dankelley closed 8 years ago

dankelley commented 8 years ago

We have topoWorld just sitting there, all lonely and such ... why not draw some contours? I'll do that, with an option drawIsobaths to turn it off. The only question is whether to have the default for drawIsobathys be TRUE or FALSE.

Oh, and maybe the arg name is in question, too. I started this comment writing drawBathymetry, but my proposal is to draw contours, so we are really drawing isobaths.

dankelley commented 8 years ago

This is implemented now in develop commit 7f5d7920ccff7e31ea1aa70e0a851cf999227a20

Thus,

library(oce)
plot(read.ctd.odf("/data/flemishCap/CTD/2013/ODF/D021A011.ODF"),drawIsobaths=TRUE)

yields as below.

screen shot 2016-08-09 at 8 23 11 am

dankelley commented 8 years ago

Q FOR CLARK -- The results look quite useful, and I am now thinking of making the default value of drawIsobaths be TRUE. What do you think, Clark?

richardsc commented 8 years ago

I'm going to give a tentative "yes", because I agree that the results look pretty useful, but I have some questions about interface (which should maybe go in their own issue).

Do we want to thing about having an interface (e.g. arguments) to control the isobath plotting? Things like:

I know it opens up a can of worms doing that, and it's obviously not worth doing too much work for future use cases, but I can see people (i.e. myself) wanting to be able to tweak the isobath plotting with a couple of arguments.

dankelley commented 8 years ago

I had the same thought, but then what we have is that this overview-type plot is going to have to have a whole lot of extra arguments for one panel, and they could contradict those in other panels (e.g. col). So it could be colMap, lwdMap, levelsMap (and nlevelMap) and so on and so on. I worry that the arg list will just get too long. (I already think it's daunting for the user.) The other thing is that in a lot of cases, people will want to use their own topo datasets (the one in oce is coarse). Given all of this, I decided to just put in a basic default with the expectation that users can add things specifically if they want later, with contour or mapContour.

For the user, the overhead of doing

draw(ctd, which='map')
load('mytopo.rda')
contour(mytopo[['longitude']], mytopo[['latitude']], -mytopo[['z']], ETC)

seems not all that much harder than

draw(ctd, which='map', ETC)

and it might actually be easier because the user already knows the args that contour uses, whereas the user will need to learn the alternatively-named args that plot,ctd-method uses.

This is why I came down on the side of simple. It's all open to discussion, of course! Maybe, as you say, this is another issue, since I've already addressed the issue as stated in the title. (If you want to start an issue and copy your notes above, I can of course copy my notes and then delete the present comment)

dankelley commented 8 years ago

I changed the default to be not drawing them. That way, old code won't make surprising output that will force users to read documentation. Speaking of documentation, in the ?"plot,section-method" docs, there is now an example of adding depth contours manually.

I am going to close this, since the task expressed in the title has been completed. Of course, Clark and I can continue discussing it here ... and the discussion of new arguments could be a new issue, if Clark wishes.