bugGNU Octave - Bugs: bug #64991, polar doesn't populate rtick with...

 
 

bug #64991: polar doesn't populate rtick with center tick value

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  Wed 06 Dec 2023 10:16:42 PM UTC
   
 
Category:  Plotting Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 8.4.0 Operating System:  * Any
Fixed Release:  10.1.0 (current default) Planned Release:  10.1.0 (current default)
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 15 Dec 2023 02:57:23 AM UTC, comment #9: 

that change seems to work and doesn't cause any other issues. pushed as
https://hg.savannah.gnu.org/hgweb/octave/rev/db0e70a46dc0

Nicholas Jankowski <nrjank>
Group Member
Wed 13 Dec 2023 06:48:22 PM UTC, comment #8: 

Attached is a quick changeset that doesn't display the rtick value of 0, but doesn't alter the 'rtick' property.

Interestingly, polarplot on Matlab does not display small values that are lower than 1e-9 times the largest value in rticks.  That looks like the precision of a single value which is what OpenGL uses.

(file #55447)

Rik <rik5>
Group administrator
Wed 13 Dec 2023 05:50:58 PM UTC, comment #7: 

i just noticed that the change causes the center value to always be displayed.  somehow missed that earlier. I may peek at it some more to see if the original behavior can be maintained where polar never shows center label.  polarplot does when specified, so it's a bit of an arbitrary nitpick since we're already operating outside of exact compatibility .

Nicholas Jankowski <nrjank>
Group Member
Wed 13 Dec 2023 05:40:21 PM UTC, comment #6: 

Reviewed and made a few tweaks in this changeset https://hg.savannah.gnu.org/hgweb/octave/rev/2cf4c5bfc410.

The only important change was that a %!test block had several groups of tests and at one point there was a blank line in between the groups.  The blank line ends the %!test block so the remaining tests just looked like commented out code.  I fixed that.

Marking bug as Fixed and closing report.

Rik <rik5>
Group administrator
Tue 12 Dec 2023 09:18:08 PM UTC, comment #5: 

pushed change to default so that polar will create the 0 value in rtick and updated associated BIST
https://hg.savannah.gnu.org/hgweb/octave/rev/a6b7c5620240

marking as ready for test.

Nicholas Jankowski <nrjank>
Group Member
Sat 09 Dec 2023 01:12:02 AM UTC, comment #4: 

made a set of BISTs for the current version of polar, verified the pass a make check and pushed to stable as
https://hg.savannah.gnu.org/hgweb/octave/rev/a3e17876a05b

will work up feature change to polar and add/adjust tests as necessary for dev.

Nicholas Jankowski <nrjank>
Group Member
Thu 07 Dec 2023 05:44:32 PM UTC, comment #3: 

sounds good. in the process I've been writing up some tests for (currently test free) polar.  I'll separate those so they after verification they could be pushed to stable.

Nicholas Jankowski <nrjank>
Group Member
Thu 07 Dec 2023 04:57:38 PM UTC, comment #2: 

I changed the category from "Incorrect Result" to "Matlab Compatibility" since there is nothing incorrect about excluding the zero point, just a choice.

I do agree that the fix is simple, Matlab-compatible, and worth doing.

Because this is more of a feature request, and doesn't cause an error in Octave, I would put the changeset on the development branch rather than the stable branch.

Rik <rik5>
Group administrator
Wed 06 Dec 2023 10:22:27 PM UTC, comment #1: 

matlab polarplot default behavior for comparison:


>> t = [0:15:180]*pi/180;
>> r = sin(t);
>> polarplot(t,r)
>> get(gca,'rtick')

ans =

         0    0.2000    0.4000    0.6000    0.8000    1.0000


Nicholas Jankowski <nrjank>
Group Member
Wed 06 Dec 2023 10:16:42 PM UTC, original submission:  

rtick and ttick work on polar plots by pulling the appropriate values from xtick or ytick, and inserting those into the appropriate property in the polar object.  i noticed that rtick doesn't contain the 'zero' value when the plot is created, but if the rtick values are manually specified later with 'rticks', it contains and returns that as part of the tick range.

Our implementation for polar operates a bit outside of any direct compatibility concerns, but in matlab 'rticks' on polarplot objects always returns the center (zero) value as part of rtick points.

Even though Matlab does not permit rticks to apply to polar plots (which have no rtick property) the thought on extending octave with those properties/functions seemed to be that that later when polarplot objects are implemented in octave, the same functions could just be overloaded there and behavior should be consistent/compatible.

I'd recommend we update rtick to always return the 0/center value. It turns out it's a very simple one character change in polar, changing line 176 calculation of the lower index from > to >= so it also pulls the 0 value from xtick. I.e:


minidx = find (xtick >= 0, 1);


behavior before change:



>> t = [0:15:180] * pi / 180;
>> r = sin (t);
>> polar (t, r)
>> get (gca, 'rtick')
ans =

   0.5000   1.0000

>> rticks (gca, [0 .5 1])
>> get (gca, 'rtick')
ans =

        0   0.5000   1.0000


behavior after change:


>> t = [0:15:180] * pi / 180;
>> r = sin (t);
>> polar (t, r)                  ^
>> get (gca, 'rtick')
ans =

        0   0.5000   1.0000

>> rticks (gca, [0 .5 1])
>> get (gca, 'rtick')
ans =

        0   0.5000   1.0000


i'll attach a patch to do that once this generates a bug number, check demos and maybe add a couple tests for polar.

would also like to verify that this is something that should go on default, not stable (for 9.1), and will write a news note accordingly.

Nicholas Jankowski <nrjank>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #55447:  rtick0.cset added by rik5 (839B - application/octet-stream)

 

Depends on the following items: None found

Digest:
   patch dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by nrjank (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-12-15 nrjank Dependencies- patch #10408 is dependent
    2023-12-13 rik5 Attached File- Added rtick0.cset, #55447
    2023-12-13 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 10.1.0 (current default)
    2023-12-12 nrjank StatusConfirmed Ready For Test
    2023-12-07 rik5 Item GroupIncorrect Result Matlab Compatibility
        StatusNone Confirmed
        Planned ReleaseNone 10.1.0 (current default)

    Back to the top

    Powered by Savane 3.13-bb6a.
    Corresponding source code