bugGNU Octave - Bugs: bug #48533, "grid on" inconsistencies

 
 

bug #48533: "grid on" inconsistencies

Submitter:  Markus Mützel <mmuetzel>
Submitted:  Sat 16 Jul 2016 05:07:58 PM UTC
   
 
Category:  Plotting Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 31 Aug 2016 04:52:47 AM UTC, comment #9: 

I pushed your final cset for grid.m here (http://hg.savannah.gnu.org/hgweb/octave/rev/22f22d53f135).  Closing report (again!).

Rik <rik5>
Group administrator
Tue 30 Aug 2016 05:12:10 PM UTC, comment #8: 

Rik, that was exactly what needed to be changed.
The attached changeset also turns the minor grid off for semilog plots when "grid" is called the second time and adds the same set of tests for semilogy that was already added for plot.
Additionally, I added a demo for the changes in bug #48528. That should probably better go into graphic.cc but - as I understand - demos in .cc-files cannot be run, and "grid.m" probably fits best.

(file #38390)

Markus Mützel <mmuetzel>
Group administrator
Tue 30 Aug 2016 03:51:44 PM UTC, comment #7: 

Yes, it seems that the the status of the axis (linear vs. log) should be checked first before turning on the minor grid.

Rik <rik5>
Group administrator
Tue 30 Aug 2016 03:30:13 PM UTC, comment #6: 

Recent changes to grid improved it significantly. It is even more Matlab compatible than what I proposed in comment #0. Nevertheless, I don't think this bug is solved completely. Calling "grid on" twice for semi-log plots still leads to an inconsistent result:

clf;
semilogy (1:100);
grid on
assert (get (gca, "xgrid"), "on")
assert (get (gca, "ygrid"), "on")
assert (get (gca, "xminorgrid"), "off")
assert (get (gca, "yminorgrid"), "on")  # This is actually what Matlab seems to do as well.
grid on
assert (get (gca, "xgrid"), "on")
assert (get (gca, "ygrid"), "on")
assert (get (gca, "xminorgrid"), "off")  # This assert fails
assert (get (gca, "yminorgrid"), "on")


The second call should not turn on the minor grid in x-direction (linear axis).

Also, see Matlab's behavior for a linear plot with minor grid on in one direction (the same holds for semilogx plots):

>> figure
>> ha = axes();
>> plot(1:10)
>> grid on
>> set(ha, 'XMinorGrid', 'on')
>> grid on
>> get(ha, 'YMinorGrid')
ans =
off


It seems that this is not (any longer?) necessary to follow Matlab, either. The logic should be removed that turns all minor grids on if only one minor grid is "on" before calling "grid on".
If nobody objects, I can provide a changeset probably within the next few days.

Markus Mützel <mmuetzel>
Group administrator
Mon 22 Aug 2016 05:05:12 PM UTC, comment #5: 

@Andy: I tested your cset and made one small change: 'grid off' now turns off both grids for logarithmic axes.  I pushed the cset here (http://hg.savannah.gnu.org/hgweb/octave/rev/c381f78e79bf).  Fixed, closing report.

Rik <rik5>
Group administrator
Sat 20 Aug 2016 04:25:55 PM UTC, comment #4: 

I really like the 'minor-auto' feature for logarithmic plots.  Let's be better than Matlab in this regard.

Rik <rik5>
Group administrator
Sat 20 Aug 2016 02:28:12 PM UTC, comment #3: 

How do we want to proceed with the "minor-auto" feature for logarithmic axes which, as far as I understand, is Octave specific.

Automatically enabling minorgrid on log axes with "grid on" looks like a nice feature to me.

Attached my last patch which decouples major and minor grid but leaves the "minor-auto" feature for lag axes.

(file #38290)

Andreas Weber <andy1978>
Group Member
Tue 16 Aug 2016 09:22:38 PM UTC, comment #2: 

I think at some point we were actually following Matlab behavior.  I seem to remember that with no grid, 'grid minor' would turn both the major and minor grid on.

But, if Matlab has changed its behavior then we can too.  It sort of makes sense to be able to control the major and minor grids independently.

Rik <rik5>
Group administrator
Wed 20 Jul 2016 06:51:21 AM UTC, comment #1: 

I think the logic in "grid.m" is some hack to work around bug #48528.
I posted a changeset there that should fix that properly for the opengl toolkits. I will wait for a fix here to see when/if the other bug is acceptable.

Markus Mützel <mmuetzel>
Group administrator
Sat 16 Jul 2016 05:07:58 PM UTC, original submission:  

In Matlab, "grid minor" toggles the drawing of only the minor grid. In Octave, the same command also turns the major grid on. That means "grid minor" in Octave is equivalent to "grid on, grid minor" in Matlab.

Additionally in "grid.m", there is some logic that turns the major grid off for logarithmic axes when calling "grid on" after "grid minor". That also seems wrong to me:

clf
semilogy (1:100)
grid minor
grid on
get (gca, 'ygrid')
ans = off


This also leads to different results when calling "grid on" twice:

clf;
semilogy (1:100);
grid on
assert (get (gca, "xgrid"), "on")
assert (get (gca, "ygrid"), "on")
assert (get (gca, "xminorgrid"), "off")
assert (get (gca, "yminorgrid"), "off") % This assert fails
grid on
assert (get (gca, "xgrid"), "on")
assert (get (gca, "ygrid"), "on") % This assert fails
assert (get (gca, "xminorgrid"), "off") % This assert fails
assert (get (gca, "yminorgrid"), "off") % This assert fails


If you do not object, I would like to re-write "grid" to closer resemble Matlab where imho it behaves more consistently obeying the following rules:
"grid": toggle only major grid
"grid on": turn only major grid on
"grid minor": toggle only minor grid
"grid off": turn minor and major grid off.
"grid minor on" and "grid minor off" are not Matlab compatible. Anyway, I would like to keep that syntax in because I see where it could be useful.

Markus Mützel <mmuetzel>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #38290:  grid_andy.diff added by andy1978 (5KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by andy1978 (Updated the item)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (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 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-08-31 rik5 StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2016-08-30 mmuetzel Attached File- Added bug48533_grid_on_twice.patch, #38390
    2016-08-30 rik5 StatusFixed In Progress
        Open/ClosedClosed Open
    2016-08-22 rik5 StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2016-08-20 andy1978 StatusPatch Submitted In Progress
    2016-08-20 andy1978 Attached File- Added grid_andy.diff, #38290
    2016-08-16 rik5 StatusNone Patch Submitted

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code