[rkward-devel] Request for feedback on plot history

[Prasenjit Kapat]

Hi,

On Wed, Sep 1, 2010 at 4:31 PM, Thomas Friedrichsmeier
<thomas.friedrichsmeier at ruhr-uni-bochum.de> wrote:
> Hi,
>
> great work, overall! This is pretty sophisticated stuff, and that also means
> there's a lot of details to think about and test. I'm not sure I've seen
> everything, but I don't want to let you wait much longer. So here are the
> notes I have taken so far in no particular order.
>
> I'll post again, if / when I find more.

Thanks for the inputs... I really need these... This whole plot
history thing has lot of nuances, so the more feed back the better...

> Warning message:
> In push.pop.and.record(which.pop = 1, deviceId = deviceId, this.plot.is.new =
> TRUE) :
>  Max length reached, popping out the first plot.
>
> Suggest to remove this one. After the limit is reached, you will see it for
> every single plot, which is probably more annoying than helpful.
>
> In contrast, the "larger than size limit" warning should only be triggered
> once in a while, so I think it is good to keep it.

Done. BTW, the printPars () calls will also be commented out / removed
before the final version. I'll keep it in till the "Add to history"
feature is ironed out.

> Is it really a good idea to synchronize the plot windows that show the same
> history position? I.e. if two separate windows each show the first plot in
> history, and that is removed, should the other window really switch to the
> next plot, too? Or should that now be considered a "fresh" plot?
>
> I could see the merit in both, but the current behavior was somewhat
> surprising to me at first.

Yeah, that was something on my mind too, but let us stick with this
for now. It can be changed to something more "intuitive" later.

In any case, the way I look at it is this: what you are removing is a
plot from the history not just from the displayed device. I'll take a
note of this and we can come back after 0.5.4.

> Should dev.set() always set newPlotExists for the respective device to true?
> In case additions are made to that plot? E.g. produce a plot, add it to
> history, then add a grid to that plot, then use previous / next actions ->
> grid is gone.

Yes, that is an inherent problem with all the secondary base graphics
functions: points, lines, abline, mtext, grid, title, axes, ....
We would not, as of current implementation, know when such a function
was called. At this stage the user should click the "+" button to save
(overwrite and not append) the plot manually.

Letting dev.set () always set newPlotExists to TRUE will force
recording with each and every arrow movement
(first/previous/next/last). I am a little reluctant in doing this. How
much of an overload would that be? Would the "browsing" feel sluggish?
May be not, I'll give it a try.

One thing that I had in mind was to use the assignInNamespace trick
for _all_ of these secondary functions; although it may work only for
some and not all of them. But this approach seems kind of brute force!

Let me see what I can do.

> Extracting the title / axis labels: Cool stuff! I no longer thought this was
> possible at all. However, currently, the code bails out in case the title has
> been re-set using title(). In this case there is more than one .Primitive
> ("title") in the recorded plot.

Ah... how cruel is that! This can be fixed (partially).

> Probably makes more sense to add this info in a drop-down list of plots in the
> history (your point c). For the plot that is currently shown, info on titles,
> etc. is not really interesting. It is interesting for browsing the plots which
> are *not* currently visible, however. I can try to add something, but I'm not
> sure how soon.

Precisely. I just wanted to have a stub which can be used later. If
you so prefer, I can comment out the entire "info" codes for this
release.

> Of course for lattice plots, the story may be different, as more info is
> potentially available. But then - personally - I think it would probably be a
> good idea to trim that down to some very basic information, too, which is
> probably titles and axis labels, again?

Right, although I'll like to keep at least the main part of the call
which shows the function and the formula used. Again, this can be
delayed for the next version.

> Either way, we can leave this as is for the current release.

See next comment.

> Toolbar is crowded. I'd suggest to remove first/last plot, remove plot, clear
> history, and info buttons from the toolbar by default.

I would like to keep "remove plot" on the toolbar, others can go, no problem ;)

> Could you check the code for parts which are no longer needed and remove
> those? .verify.history.limits() appears to be one unused function.

I'll check these. The verify.hist.limits () fn is used in
settings/rksettingsmoduleoutput.cpp ~199

Thanks again,
-- 
Prasenjit