bugGNU Octave - Bugs: bug #49794, display vs. disp functions

 
 

bug #49794: display vs. disp functions

Submitted by:  Amro <amro_octave>
Submitted on:  Wed 07 Dec 2016 04:43:02 PM UTC  
 
Category: Octave FunctionSeverity: 2 - Minor
Priority: 5 - NormalItem Group: Matlab Compatibility
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Wed 29 Mar 2017 06:40:15 PM UTC, comment #30:

I made an additional change so that Octave never calls the display function with more than one input argument since this is not documented Matlab behavior and it was causing trouble for existing Octave classes. It might be possible to check whether a display function for a class is defined to accept one or two arguments and call it differently based on this determination, but I opted against that since this feature is undocumented in Matlab.

I modified the documentation for the built-in display function so that it doesn't mention the two-argument version, but I left it as before so that things like

will work (as they do in Matlab).

I'll fix any display methods in Octave so that they use inputname instead of a second argument.

Here's the changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/7d4ca8c01bbb

See also bug #50640

John W. Eaton <jwe>
Project Administrator
Wed 08 Feb 2017 05:35:01 PM UTC, comment #29:

The extra second input for display() should definitely be documented.

Since you are overloading the existing display() function in a new class, it seems like it must have the same function signature as the function it overrides. In this case, the function accepts two inputs so InputParser's version of display should also accept two inputs. It doesn't have to do anything with the second input if it doesn't want to. The choice of what to implement is left to the programer.

Rik <rik5>
Project Administrator
Wed 08 Feb 2017 12:15:51 PM UTC, comment #28:

In comment #3 there was told, that Matlabs "display" accepts an undocumented second input with the printed variable name. I just figured out the hard way, that Octave 4.3.0+ does the same.

While refactoring inputParser, I noticed that calls like

resulted in an ugly error. After reading this bug report I know why:

Carnë and I never expected display to have this feature, thus long existing code like in inputParser:

Does not work any longer. Now some questions how to handle this:

1) Just change to (nargin > 2), that actually works. For me the question is, does this behavior remain (is it "stable")?
2) Above + add a note to the docstring of display (I hate undocumented stuff).

About the other issue in print_usage I care in another bug report.

Kai Torben Ohlhus <siko1056>
Project Member
Sat 17 Dec 2016 05:31:53 PM UTC, comment #27:

Yeah, apparently Matlab displays the first dispatch type it encounters. I guess this also means that in Matlab, disp is a generic function and does not have any dispatch types declared?

OK, I checked in the following change to make it compatible with 2016b. Gah!

http://hg.savannah.gnu.org/hgweb/octave/rev/b28801182c08

Thanks.

John W. Eaton <jwe>
Project Administrator
Sat 17 Dec 2016 04:00:07 PM UTC, comment #26:

Already shown in my previous comment #24.

The missing ones are:

The last one might seems weird ("display" from "cell"). So here's the list of all overloads:

Amro <amro_octave>
Sat 17 Dec 2016 03:52:49 PM UTC, comment #25:

Seriously? They changed that from 2016a to 2016b? Was that documented in the release notes?

Can you please run the following commands in Matlab and upload the transcript here?

John W. Eaton <jwe>
Project Administrator
Sat 17 Dec 2016 03:50:28 PM UTC, comment #24:

Here's the same running in R2016b:

Apparently this was a bug in "display" which got fixed in R2016b.

You might have also noticed the "header" printed on top of native types. This was added in R2016b, and documented in the release notes:
https://www.mathworks.com/help/matlab/release-notes.html?rntext=Command+Window&startrelease=R2016b&endrelease=R2016b&category=Desktop&groupby=release&sortby=descending&searchHighlight=Command+Window

Amro <amro_octave>
Sat 17 Dec 2016 03:41:26 PM UTC, comment #23:

I ran all my examples in the latest MATLAB version R2016b.

Amro <amro_octave>
Sat 17 Dec 2016 03:38:59 PM UTC, comment #22:

Yes, I misspoke in comment #0 when I said that "display" will print "ans =" for expressions, it doesn't. I rectified that in comment #3. Sorry for the confusion.

Amro <amro_octave>
Sat 17 Dec 2016 03:38:23 PM UTC, comment #21:

Also, with the same version using a struct for the example just to be sure there isn't something funny going on with them:

John W. Eaton <jwe>
Project Administrator
Sat 17 Dec 2016 03:34:56 PM UTC, comment #20:

Rik: What version of Matlab was used to run the test? Maybe the behavior has changed? For 2016a, it looks like display always prints the header line (variable name or "ans") and disp does not.

John W. Eaton <jwe>
Project Administrator
Sat 17 Dec 2016 12:51:52 AM UTC, comment #19:

See comments #2, #3. For example,

Matlab only provides "ans =" when called from the parser, but when called through the function display() it is omitted.

Pretty minor, but we've come this far in our quest for compatibility, why not a few more inches.

Rik <rik5>
Project Administrator
Fri 16 Dec 2016 10:12:28 PM UTC, comment #18:

If I understand the original submission correctly, that's what Matlab does for disp, not for display and Octave was getting it wrong. So I think we need to fix the test instead.

John W. Eaton <jwe>
Project Administrator
Fri 16 Dec 2016 09:10:53 PM UTC, comment #17:

There is now a failing test during 'make check'.

According to the test results from comment #0, Matlab suppresses the 'ans = ' when asked to display a raw value.

Rik <rik5>
Project Administrator
Fri 16 Dec 2016 05:40:33 AM UTC, comment #16:

I checked in the following changeset to refactor the display and disp functions:

http://hg.savannah.gnu.org/hgweb/octave/rev/9baa19102908

Now display calls disp and display is called when Octave displays the result of an expression. If I missed any places where this should happen, let me know and I'll try to fix asap.

You may also overload display, disp, and fdisp using class methods (for example, @double/display.m).

John W. Eaton <jwe>
Project Administrator
Mon 12 Dec 2016 09:18:56 PM UTC, comment #15:

Rik: It might be fairly simple to arrange for Octave to call functions for various built-in classes (double, single, cell, struct, etc.). The dispatch mechanism is already there. The only thing that is really missing is associating existing built-in functions with the various classes in the symbol table. I'll try to write something more detailed on the maintainers list.

John W. Eaton <jwe>
Project Administrator
Sat 10 Dec 2016 11:41:56 AM UTC, comment #14:

Rik: Yes, it works if you do something like

but

alone does not call display. I think it should. So the dispatch seems backward to me.

I'm thinking of something more like the attached patch that always calls the "display" method for an object, and that simply prints a name tag and then calls disp to actually print the values.

But the attached patch is not complete, and certainly not appropriate for stable. We would also need to install "display" and "disp" methods for the built-in classes (double, single, logical, etc.) so that it would not be easy to screw up displaying those types simply by creating a global function called "display".

So, for stable, I guess it is OK to fix the bug in whatever way works with the fewest surprises. But I don't think we can be fully compatible until we are calling "display" directly when an expression doesn't end with a semicolon, and then "display" should call "disp", same as Matlab does.

(file #39193)

John W. Eaton <jwe>
Project Administrator
Fri 09 Dec 2016 10:20:38 PM UTC, comment #13:

I did some more testing, and it seems that Octave calls the correct overloaded functions. So dispatching works correctly as far as I can tell.

I've used the following classes to cover all four cases of overloading or not disp/display:

Here is the output in Octave 4.2.0:

Here is the output in MATLAB R2016b:

To get rid of the errors in Octave, I'd have to define "disp" method as "function varargout = disp(obj)", and return a string if requested, instead of always printing.

As for MATLAB, apparently it treats "display" in a special manner.. You can how it errors when called with "builtin" versus directly. I searched the docs and there was this paragraph mentioned:

  • MATLAB adds default methods named disp and display to all MATLAB classes that do not implement their own methods with those names. These methods are not visible, but create the default simple display.

One more note; MATLAB does suggest to never overload "display" but instead to overload "disp". That's because the default "display" function calls "disp" to handle the actual display of the values, then simply prepends a header when appropriate (kinda like the version I suggested in comment #7)

Sorry for the long post!

Amro <amro_octave>
Fri 09 Dec 2016 09:25:03 PM UTC, comment #12:

@jwe: Agreed. There is a long term structural change needed. In the mean time, removing the call to error() gets us pretty close.

Regarding dispatch, Octave does reach display.m for a classdef class which doesn't define that method. At least that's what the test class "blah" shows.

I stopped throwing an error for class objects without a display method here (http://hg.savannah.gnu.org/hgweb/octave/rev/691d81fc885b).

Rik <rik5>
Project Administrator
Fri 09 Dec 2016 09:07:58 PM UTC, comment #11:

To properly fix this problem, I think we need to untangle the disp, display, and octave_value::print methods so that they work in a way that is both Matlab compatible and predictable.

Currently, when an object is displayed (for example, by typing it's name at the command prompt without a trailing semicolon), Octave calls the function octave_value::print_with_name. That function simply dispatches to the actual octave_value type. If the type is class or classdef, then the dispatching currently happens inside the octave_class::print_with_name or octave_classdef::print_with_name methods. Maybe that's not the right thing to do. Maybe we should just be calling display for the object and let Octave's normal dispatch rules choose which actual function to call. As it is now, I don't think you'll actually end up in the default display.m function for a classdef object, even if there is no display method, will you? Does that actually happen? If so, how?

The larger issue here is really how dispatching works generally. We currently have different ways to handle it for built-in objects and user-defined classes. So instead of just defining methods for @double or @struct, we handle those specially, and then look up methods from inside the octave_class and octave_classdef class methods that could be overloaded. Maybe we need to think about a different approach.

John W. Eaton <jwe>
Project Administrator
Fri 09 Dec 2016 09:00:57 PM UTC, comment #10:

That looks correct to me, given the current implementation of disp for objects in Octave.

You see MATLAB's default disp/display goes a bit further by printing a list of public properties for objects and their values (format similar to struct display), but this would be a separate feature request.

For reference, here is the output in MATLAB for your example:

Amro <amro_octave>
Fri 09 Dec 2016 08:15:32 PM UTC, comment #9:

Bringing over comments from Julien at bug #49753

The output from an object which does not define a display method is something, rather than an error, in Matlab.

This matches Amro's observation in comment #6.

This is easy enough to achieve by removing the input validation on isobject. If I comment out those lines, and use the following blank classdef file blah.m

then I get the following results

Does that look right?

Rik <rik5>
Project Administrator
Fri 09 Dec 2016 07:29:29 PM UTC, comment #8:

maybe replace disp with fprintf for the variable name, so that it is similar to Octave's default output which is on one line.

Amro <amro_octave>
Fri 09 Dec 2016 07:25:17 PM UTC, comment #7:

I was thinking this:

Amro <amro_octave>
Fri 09 Dec 2016 06:57:38 PM UTC, comment #6:

Thanks. May I ask why you throw an error for objects? MATLAB does provide a default disp/display for classes even if they don't implement one.

Amro <amro_octave>
Fri 09 Dec 2016 05:42:07 PM UTC, comment #5:

I checked in a change on the stable branch (http://hg.savannah.gnu.org/hgweb/octave/rev/3bc6e9a71d9e). This will be a part of the 4.2.1 bug fix release.

In general, we don't try to follow any undocumented Matlab syntax as that could change without warning. There's no reason not to implement CustomDisplay, but someone would have to take an active interest and code it up for Octave.

Rik <rik5>
Project Administrator
Fri 09 Dec 2016 04:17:13 PM UTC, comment #4:

It's really related to this report, but it's also worth mentioning that MATLAB introduced a new system for customizing display of objects (classdef objects).

This includes a new "details" function as well as a "CustomDisplay" interface which classes can inherit/implement.

There is a good amount of documentation for it: https://www.mathworks.com/help/matlab/custom-object-display.html

Amro <amro_octave>
Fri 09 Dec 2016 04:13:17 PM UTC, comment #3:

Right, sorry for not being precise.

Basically "display" only shows the variable name if the input is an existing variable, no name is printed for expressions (not even "ans" as I mistakenly stated before). This is of course followed by the actual value.

Here's a doc page that further describes disp vs. display: https://www.mathworks.com/help/matlab/matlab_oop/displaying-objects-in-the-command-window.html

Here are some examples in MATLAB to show the different cases:

Note: I use "format compact" by default in MATLAB. This tends to produce less blank lines as opposed to "format loose". Now Octave have both these format settings, but it doesn't seem to have an effect on the output, and seems to always use the compact form. I wanna keep the bug report focused on one issue, so I'm not too concerned about this aspect here :)

Finally note that "display" in MATLAB has a second undocumented input, it can be used to override the variable name printed. Example:

Amro <amro_octave>
Fri 09 Dec 2016 03:52:44 PM UTC, comment #2:
Guillaume <gyom>
Fri 09 Dec 2016 03:06:25 PM UTC, comment #1:

How does Matlab handle a built-in type which doesn't have a variable name? Can you try this in Matlab and report back?

Rik <rik5>
Project Administrator
Wed 07 Dec 2016 04:43:02 PM UTC, original submission:

The function "display" in Octave behaves a bit differently than the one in MATLAB. According to MATLAB docs:

  • disp(X) displays the value of variable X without printing the variable name. Another way to display a variable is to type its name, which displays a leading "X =" before the value.
  • display(X) is called by MATLAB for the object X when a statement is not terminated by a semicolon.

-verbatim-

Here is an example in MATLAB R2016b using builtin types:

The same in Octave 4.2.0:

I'm guessing Octave should use "inputname" inside "display" to first print the variable name or "ans" in case the input was an expression, then use "disp" for the rest.

Also note that the current implementation in Octave does something like this (related to bug #49753):

str = disp(obj);
...
disp(str)

so it ends up displaying an extra new line.

Amro <amro_octave>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #39193:  diffs.txt added by jwe (5KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by siko1056 (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by gyom (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by amro_octave (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 10 Dec 2016 11:41:56 AM UTCjweAttached File-=>Added diffs.txt, #39193
    Fri 09 Dec 2016 05:42:07 PM UTCrik5StatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Fri 09 Dec 2016 03:06:25 PM UTCrik5Severity3 - Normal=>2 - Minor
      StatusNone=>Confirmed
      Release4.2.0=>dev

    Back to the top


    Powered by Savane 3.1-cleanup1