[changeset] colorbar bug

View: New views
5 Messages — Rating Filter:   Alert me  

[changeset] colorbar bug

by Ben Abbott :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

This changeset addresses two bugs I encountered.

(1) Two colorbar commands produced two colorbars.

(2) colorbar ("off") did not turn off the colorbar.

Demos are included which verify the intended behavior.

This fix was trivial, and has been pushed to savannah.

Ben



changeset-colorbar.patch (3K) Download Attachment

Re: [changeset] colorbar bug

by dbateman :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Ben Abbott wrote:
This changeset addresses two bugs I encountered.

(1) Two colorbar commands produced two colorbars.

(2) colorbar ("off") did not turn off the colorbar.

Demos are included which verify the intended behavior.

This fix was trivial, and has been pushed to savannah.

Ben
I'm not sure (1) is a bug. Yes I noticed this behavior when I reimplement colorbar in 3.2, but kept it as that is similar behavior to matlab when I tested it. However, I think its probably better to only have a single colorbar so the fix is fine.

D.
 

Re: [changeset] colorbar bug

by Ben Abbott :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


On Mar 1, 2009, at 1:52 PM, dbateman wrote:

>
> Ben Abbott wrote:
>>
>> This changeset addresses two bugs I encountered.
>>
>> (1) Two colorbar commands produced two colorbars.
>>
>> (2) colorbar ("off") did not turn off the colorbar.
>>
>> Demos are included which verify the intended behavior.
>>
>> This fix was trivial, and has been pushed to savannah.
>>
>> Ben
>>
>
> I'm not sure (1) is a bug. Yes I noticed this behavior when I  
> reimplement
> colorbar in 3.2, but kept it as that is similar behavior to matlab  
> when I
> tested it. However, I think its probably better to only have a single
> colorbar so the fix is fine.
>
> D.

I should have checked what Matlab does.

 >> clf
 >> set(0,'handleVisibility','on')
 >> plot(1:10)
 >> hc1 = colorbar

hc1 =          173

 >> hc2 = colorbar

hc2 =          173

Thus Octave's present implementation shoud not delete colorbar replace  
it with a new one, but should update it.

When given a new location, a second colorbar is created.

 >> hc3 = colorbar('westoutside')

hc3 =          203

I did not expect this behavior. The fix should be trivial. I'll look  
at it.

Ben



Re: [changeset] colorbar bug

by Ben Abbott :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


On Mar 1, 2009, at 7:57 PM, Ben Abbott wrote:

>
> On Mar 1, 2009, at 1:52 PM, dbateman wrote:
>
>>
>> Ben Abbott wrote:
>>>
>>> This changeset addresses two bugs I encountered.
>>>
>>> (1) Two colorbar commands produced two colorbars.
>>>
>>> (2) colorbar ("off") did not turn off the colorbar.
>>>
>>> Demos are included which verify the intended behavior.
>>>
>>> This fix was trivial, and has been pushed to savannah.
>>>
>>> Ben
>>>
>>
>> I'm not sure (1) is a bug. Yes I noticed this behavior when I  
>> reimplement
>> colorbar in 3.2, but kept it as that is similar behavior to matlab  
>> when I
>> tested it. However, I think its probably better to only have a single
>> colorbar so the fix is fine.
>>
>> D.
>
> I should have checked what Matlab does.
>
> >> clf
> >> set(0,'handleVisibility','on')
> >> plot(1:10)
> >> hc1 = colorbar
>
> hc1 =          173
>
> >> hc2 = colorbar
>
> hc2 =          173
>
> Thus Octave's present implementation shoud not delete colorbar  
> replace it with a new one, but should update it.
>
> When given a new location, a second colorbar is created.
>
> >> hc3 = colorbar('westoutside')
>
> hc3 =          203
>
> I did not expect this behavior. The fix should be trivial. I'll look  
> at it.
>
> Ben
David, the job is a bit more complicated than I'd expected. It appears  
to me that a rewrite of update_colorbar is needed.

        addlistener (ax, "dataaspectratio", {@update_colorbar_axis, cax})
        addlistener (ax, "position", {@update_colorbar_axis, cax})

Updating a colorbar axis when other colorbars are present will require  
a significant change to how this presently works. I'd (1) add the  
listener to the data axis, (2) pass the original data axis position  
through the listener, (3) have the data axis updater find all  
associated colorbars and set the position and size of data axis and  
all associated colorbar axes, (4) preserved the original data axis  
position value, by taking care that the listener is not modified as  
subsequent colorbars are added.

I'm still studying how  your present implementation is working. Does  
my approach sound proper, or can you recommend a something better?

If not, I'm not sure that respecting compatible behavior in this  
instance is important enough at the moment (there are some more  
important backend issues that need attention for the 3.2 release), so  
I'm likely to delay working further on this.

In any event, a partial changeset is attached below. It preserves the  
handle when colorbar is called multiple times for the same location,  
but only allow one colorbar per axes (a demo has been added to  
illustrate this behavior). I've also attempted to ensure that non-
gnuplot backends will work.

Can you take a quick look?

Ben










changeset-colorbar.patch (2K) Download Attachment

Re: [changeset] colorbar bug

by dbateman2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Sun, Mar 01, 2009 at 10:04:42PM -0500, Ben Abbott wrote:

>
> On Mar 1, 2009, at 7:57 PM, Ben Abbott wrote:
>
> >
> >On Mar 1, 2009, at 1:52 PM, dbateman wrote:
> >
> >>
> >>Ben Abbott wrote:
> >>>
> >>>This changeset addresses two bugs I encountered.
> >>>
> >>>(1) Two colorbar commands produced two colorbars.
> >>>
> >>>(2) colorbar ("off") did not turn off the colorbar.
> >>>
> >>>Demos are included which verify the intended behavior.
> >>>
> >>>This fix was trivial, and has been pushed to savannah.
> >>>
> >>>Ben
> >>>
> >>
> >>I'm not sure (1) is a bug. Yes I noticed this behavior when I  
> >>reimplement
> >>colorbar in 3.2, but kept it as that is similar behavior to matlab  
> >>when I
> >>tested it. However, I think its probably better to only have a single
> >>colorbar so the fix is fine.
> >>
> >>D.
> >
> >I should have checked what Matlab does.
> >
> >>> clf
> >>> set(0,'handleVisibility','on')
> >>> plot(1:10)
> >>> hc1 = colorbar
> >
> >hc1 =          173
> >
> >>> hc2 = colorbar
> >
> >hc2 =          173
> >
> >Thus Octave's present implementation shoud not delete colorbar  
> >replace it with a new one, but should update it.
> >
> >When given a new location, a second colorbar is created.
> >
> >>> hc3 = colorbar('westoutside')
> >
> >hc3 =          203
> >
> >I did not expect this behavior. The fix should be trivial. I'll look  
> >at it.
> >
> >Ben
>
> David, the job is a bit more complicated than I'd expected. It appears  
> to me that a rewrite of update_colorbar is needed.
>
> addlistener (ax, "dataaspectratio", {@update_colorbar_axis, cax})
> addlistener (ax, "position", {@update_colorbar_axis, cax})
>
> Updating a colorbar axis when other colorbars are present will require  
> a significant change to how this presently works. I'd (1) add the  
> listener to the data axis, (2) pass the original data axis position  
> through the listener, (3) have the data axis updater find all  
> associated colorbars and set the position and size of data axis and  
> all associated colorbar axes, (4) preserved the original data axis  
> position value, by taking care that the listener is not modified as  
> subsequent colorbars are added.
>
> I'm still studying how  your present implementation is working. Does  
> my approach sound proper, or can you recommend a something better?
>
> If not, I'm not sure that respecting compatible behavior in this  
> instance is important enough at the moment (there are some more  
> important backend issues that need attention for the 3.2 release), so  
> I'm likely to delay working further on this.
>
> In any event, a partial changeset is attached below. It preserves the  
> handle when colorbar is called multiple times for the same location,  
> but only allow one colorbar per axes (a demo has been added to  
> illustrate this behavior). I've also attempted to ensure that non-
> gnuplot backends will work.
>
> Can you take a quick look?


Frankly I can't imagine a good reason to have two colorbars and so if
its too hard just leave it as is after your change

D.