[rkward-devel] Request for feedback on plot history

[Thomas Friedrichsmeier]

Hi,

On Saturday 04 September 2010, Prasenjit Kapat wrote:
> Indeed. I'll need to go back to drawing board and try to implement
> things differently. Before getting into specific cases, I am
> suggesting, in the interest of the upcoming release, that let us
> ignore this plot history feature completely. Why? There is another
> core problem:
> 
> par (mfrow = c(2,2))
> plot (1,1); plot (2,1); plot (1,2); plot (2,2)
> 
> A similar problem occurs for screen calls. For example, follow one of
> the examples in ?screen. I am not sure, how / if we can avoid this! Is
> this an "acceptable" drawback for a released software? I would guess
> not.

I think you're looking at it a bit too negatively. Yes, I was aware of the 
mfrow/mfcol/mfg problem (but not of the screen() problem), and yes, this is a 
real drawback. But not a showstopper, IMO.

I mean, we are talking about corner cases to some degree in all these issues. 
The "plain" case works well, even for trellis plots, and the plot history 
feature adds real value in this case! We should try to achieve perfection over 
time, but R being the flexible beast that it is, it's pretty much impossible to 
create a fully perfect solution to any problem.

The type of problem that we should be *very* careful to avoid is making 
something worse due to new features. E.g. the user losing plots, as could 
conceivable happen with the current implementation of the remove-feature. But 
here, we're talking about a use case where the plot history simply won't be of 
much use, currently, not where it would do any harm.

Either way, I'd say, let's target a release in roughly 3-4 weeks. That will 
give us at least one more week to try to achieve a good basic solution for 
this feature. After that week we can discuss whether the remaining problems 
can be tolerated, or fixed in time for the release.

Overall, this is a really cool feature, and I'd really like to make it part of 
the release!

> Getting rid of the "remove" action is not really an option, if we want
> to impose a history length restriction. So, even if the user is not
> provided with a visible "remove" action, it still has to be
> implemented internally.

Yes. What I'm arguing is that remove should not alter any other device. If a 
plot is removed in device 2, and the same plot is still visible in device 3, 
let's just keep it in device 3, there. It should be purged from the history, 
but not from any other device besides 2. In device 3 it should now be 
considered "new".

I.e. removing a plot in one device window should never remove the plot in a 
separate device window.

Yes, in some cases this might not be what the user intends, but it's quite 
simply the safe thing to do. Besides, we are - once again - talking about a 
corner case, here, not something that would happen to users a lot.

Perhaps the same basic strategy applies for replacing plots, and for 
duplicated devices, but it's a bit tricker, there. More on that, somewhere 
below.

> Right, you have a valid point here. And it is tied to the inability to
> handle more than one new / unsaved plots. To be able to handle it,
> things will have to be implemented differently (I have an idea,
> similar to what I had earlier. But it will take some time and thinking
> to combine the two strategies together.)

I'm don't know the details of your implementation the way you do, but wouldn't 
the following be enough to address this:
- For "new" plots assume the special history position "0" (or NA, or 
whatever).
- Once plots get recorded, they will be inserted into regular history 
positions as before. In case of several "new" plots, they will be inserted 
into the history one by one in the order of device numbers.
- Naturally, .rk.graph.history.gui() will need to be made a bit smarter (or 
the C++ code). But probably it's enough to translate the special "new" 
position into history-length + 1 at this point.

> This was on my mind as well. And this is what windows does too,
> slightly differently. To implement it in a way so that the user can
> change it dynamically during a running rkward session will be tricky.
> But, to do it statically (ie needs to restart rkward) will be easy.
> I'll just have to, essentially, include the plot.new, dev.off, etc.
> wrappers inside an __if (getOption ("rk.implement.plot.history"))__
> block.

Well, I guess that would be good enough. This is just to be on the safe side, 
anyway.

> Fair enough. But it would be difficult to avoid a duplicated plot in
> history. For example consider duplicating device 2 to device 3 and the
> status of the plot on device 3 is set to "new." Now, if the next
> command from the user is a primary function call (as opposed to a
> secondary one, such as: title, points, lines, grid, ...) such as:
> plot/persp/hist/xyplot/... then the plot which was copied from device
> 2 will get recorded to the history at a new position, yet, it will be
> the exact same plot as the already (?) recorded one from device 2.
> Just another source of confusion / surprise ;)

Basically my philosophy is: Rather tolerate a duplicate plot in history than 
risk having one plot to few in history. Perhaps the user has used "Duplicate 
Device" precisely to make modifications to one copy, but to keep the other 
copy. Thus the copy would be logically different.

But on a more general note: What should be the policy when two devices show 
the same history position, and the user modifies the plot in one? I suggest 
that the plot in the other device should remain untouched, but be considered a 
"new", not yet saved plot from now on. I.e.: Modifying a plot in one device 
window should never modify the plot in a separate device window. Once again, 
this is simply the safe thing to do, even if it may not always be exactly what 
the user expected.

Well, as easy as that sounds in theory, it hinges on a reliable detection of 
when a plot has really changed. Currently we need to assume that the plot 
*may* have changed, any time the user navigates the history (among others). 
This would easily lead to a lot of confusion when using multiple devices.

But perhaps it's not that hard after all? I just tried this with the X11Cairo 
device:
    plot(1, 1)
    a <- recordPlot()
    dev.off()
    replayPlot (a)
    b <- recordPlot()
    identical (a, b)	# TRUE
    title ("something's changed")
    c <- recordPlot()
    identical (a, c)	# FALSE
I have not yet tested the windows() or quartz() devices, yet. But if the same 
thing works, there, this means we do have a reliable way to tell, whether a 
plot has been modified after all (only after calling recordPlot(), but still).

So - does this sound like a feasible strategy?

> So, what next?
> * One simpler strategy is to use separate history for individual
> devices, the way it is implemented on Mac OSX's quartz device. This
> way replaying / updating across devices is totally avoided.
> * Provide only first / prev / next / last and clear history actions,
> again, similar to quartz (), with no restriction on history length.
> Thereby, no complications due to "remove" action.

Before resorting to this, I'd give the current approach another chance. After 
all, the drawback of separate histories is that the history is lost when 
closing the device.

> Even with these, I do not see, without a more low level control on the
> device, how we can avoid the par (mfrow) and screen () issues
> mentioned on top. May be we can have a wrapper around screen ()! May
> be we can check the mfrow parameter, before calling record ()! May be
> there is "new.page" call/parameter somewhere which we can wrap around!
> May be, may be, may be.....

Yes, maybe we can find a solution for those, and maybe we cannot. But again, I 
would not see this issues as a show-stopper for an otherwise extremely useful 
feature.

Regards
Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/rkward-devel/attachments/20100905/c39fc24f/attachment.sig>