bugGNU Octave - Bugs: bug #50756, display of objects in cell arrays...

 
 

bug #50756: display of objects in cell arrays and structs is suppressed

Submitter:  Mike Miller <mtmiller>
Submitted:  Fri 07 Apr 2017 09:35:54 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Confirmed Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 08 Apr 2018 07:18:29 AM UTC, comment #9: 

So the newline appears b/c print_as_scalar() is always false for classes.  If we are going to keep the "<class sym>" style of output for 4.4, maybe we should set this to true?

I tried a patch to make that always true and then call the class's display method, assuming it will DTRT.  Results look reasonable but unfort I get segfaults when I invoke "ans".


>>> {-42, 'hi', sym('x')/2, 43, {1 2 sym('x') 4 {11 12 sym('z')/113 14}}}
ans =
{
  [1,1] = -42
  [1,2] = hi
  [1,3] = (sym)
  x
  ─
  2
  [1,4] =  43
  [1,5] =
  {
    [1,1] =  1
    [1,2] =  2
    [1,3] = (sym) x
    [1,4] =  4
    [1,5] =
    {
      [1,1] =  11
      [1,2] =  12
      [1,3] = (sym)
   z
  ───
  113
      [1,4] =  14
    }
  }
}


interval pkg:

>>> {42 {{{43 infsup(1, 2) infsup(5, [6 6.1; 6.2 6.3]) 44}}} 45}
ans =
{
  [1,1] =  42
  [1,2] =
  {
    [1,1] =
    {
      [1,1] =
      {
        [1,1] =  43
        [1,2] = ans = [1, 2]
        [1,3] = ans ⊂ 2×2 interval matrix
        [5, 6]   [5, 6.1]
   [5, 6.2001]   [5, 6.3]
        [1,4] =  44
      }
    }
  }
  [1,3] =  45
}


Note @infsup/display seems to add a few extraneous "ans".


(file #43837)

Colin Macdonald <cbm>
Sat 07 Apr 2018 01:32:14 AM UTC, comment #8: 

I'm sure lots of smart things could be done here (indenting properly for example) but for 4.4, I would be happy with minimizing regression by just printing as before but with "[1,5] = "


>> syms x
>> C = {1 x 42 2*x 'hello', [x x; x x]}
C =
{
  [1,1] =  1
  [1,2] = (sym) x
  [1,3] =  42
  [1,4] = (sym) 2⋅x
  [1,5] = hello
  [1.6] = (sym 2×2 matrix)
  ⎡x  x⎤
  ⎢    ⎥
  ⎣x  x⎦
}


Or even

>> C
C =
{
  [1,1] =  1
  [1,2] = <class sym>
  [1,3] =  42
  [1,4] = <class sym>
  [1,5] = hello
  [1.6] = <class sym>
}


The current behaviour has an undesirable newline in it:

C =
{
  [1,1] =  1
  [1,2] =
    <class sym>
  [1,3] =  42
  [1,4] =
    <class sym>
  [1,5] = hello
  [1,6] =
    <class sym>
}


So it seems like a minimal fix is removing that newline...

Colin Macdonald <cbm>
Mon 19 Mar 2018 02:34:12 AM UTC, comment #7: 

@oheim - Matlab only shows something like "1x1 foo" for an object of type "foo" in its cell array output.

Matlab cell arrays look something like this


    [1]   [2]   [1x1 obj]
    [3]   [4]   [      5]


so there is no display of object types.

If we want to be more Matlab friendly, cell arrays should be displayed in a grid format like this. See bug #43683.

Mike Miller <mtmiller>
Group Member
Sun 18 Mar 2018 10:28:12 AM UTC, comment #6: 

Yes, it is a regression. However, it never really worked. In previous versions of Octave, during output of cell arrays the overridden “display” method has been called. It was possible to find out that the content of a cell had to be printed and the cell index had to be printed by the overridden “display” method instead of the usual variable name.

For example:
https://sourceforge.net/p/octave/interval/ci/9526f0fb5f9ef5480bf595d8c4c7a9ee30a93b20/tree/inst/@infsup/display.m#l84

Also, the overridden “display” method had to indent the output, which was impossible to do correctly for nested cell arrays, because the required information was unaccessible (octave_value::current_print_indent_level).

From the implementing class's point of view it would be the simplest solution if the indentation could be done by the calling code. However, this is probably difficult to do unless you intercept and redirect stdout.

A simpler solution probably is to use the overridden “disp” method, reduce the terminal width for the duration of that call, and print the indented result with indexes.

Can you check how it should be done to be Matlab compatible?

Oliver Heimlich <oheim>
Sat 08 Apr 2017 06:42:12 PM UTC, comment #5: 

Yeah understood. We definitely do want to keep the new behavior of showing the cell indices for each entry, if we keep cell display as it is.

I'd like to also point to #40211 and #43683, which are both about implementing complete overhauls of the way cell arrays are displayed by default. If either of these are done, this may become moot.

Mike Miller <mtmiller>
Group Member
Sat 08 Apr 2017 04:39:23 PM UTC, comment #4: 

My attempt was a quick hack, not to be taken too seriously.

For example, with patch applied


x = {1;2;3}
x =
{
   1
   2
   3
}


But that is a regression from what we have today


x = {1;2;3}
x =
{
  [1,1] =  1
  [2,1] =  2
  [3,1] =  3
}


Octave probably still needs to call os.print_with_name, but maybe needs to call display() to fill the temporary buffer buf.


Rik <rik5>
Group administrator
Sat 08 Apr 2017 12:47:27 AM UTC, comment #3: 

So if a class intentionally overrides both disp and display, or doesn't override disp, is it correct that we want display(c), where c is a cell array containing an object, to call disp(x) on the object, not display(x)?

The result of this decision is that the screen output of c may appear different from the screen output of c{1}.

I'm pointing this out because both the @pyobject class (in pytave) and the @sym class (in the symbolic package) have both disp and display overrides which give slightly different output. In both cases display does call disp, but it adds a bit of information about the type of the variable in addition to its text representation.

Mike Miller <mtmiller>
Group Member
Fri 07 Apr 2017 10:36:26 PM UTC, comment #2: 

I changed the polynomial code to overload the disp function rather than the display function.  This is how things should be done.  See cset http://hg.savannah.gnu.org/hgweb/octave/rev/a5280a05b188.

However, that is only a prelude.  You are correct that the issue is that the print routine for cells in ov-cell.cc is calling print directly on an octave_value object which does not seem to invoke overloaded version of either display or disp.

I made a quick hack using feval which calls disp() on the octave_value object.  This works, but probably only jwe will know if this is the correct approach.  See the attached hack.

(file #40326)

Rik <rik5>
Group administrator
Fri 07 Apr 2017 09:45:49 PM UTC, comment #1: 

Confirmed.

Rik <rik5>
Group administrator
Fri 07 Apr 2017 09:35:54 PM UTC, original submission:  

With the changes that have been made to the way the display method is used, the following no longer shows the value of objects contained in a cell array.

In Octave 4.2.1, with the polynomial class provided in examples/code:


>> x{1} = polynomial ([1, 2, 3, 4])
x =
{
 = 1 + 2 * X + 3 * X ^ 2 + 4 * X ^ 3
}
>> x{1}
ans = 1 + 2 * X + 3 * X ^ 2 + 4 * X ^ 3


And on the default branch


>> x{1} = polynomial ([1, 2, 3, 4])
x =
{
  [1,1] =

    <class polynomial>
}
>> x{1}
ans = 1 + 2 * X + 3 * X ^ 2 + 4 * X ^ 3


If we do want the old behavior back, then I guess we need to arrange for octave_cell::print to make sure that the overloaded display method is called for each element that it contains, instead of going straight to its built-in print method.

Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43837:  cbm_50756_draft.patch added by cbm (1KiB - text/x-patch)
file #40326:  50756.diff added by rik5 (997B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by cbm (Posted a comment)
  • -email is unavailable- added by oheim (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  •  

    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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-02-26 mtmiller Carbon-CopyRemoved 80942 -
    2018-04-08 cbm Attached File- Added cbm_50756_draft.patch, #43837
    2017-04-10 rik5 Summarydisplay of objects in cell array is suppressed display of objects in cell arrays and structs is suppressed
    2017-04-07 rik5 Attached File- Added 50756.diff, #40326
    2017-04-07 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code