bugGNU Octave - Bugs: bug #52135, bar cannot plot two stacked bars

 
 

bug #52135: bar cannot plot two stacked bars

Submitter:  Marco Caliari <caliari>
Submitted:  Thu 28 Sep 2017 06:36:29 AM UTC
   
 
Category:  Plotting Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
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

Sat 14 Oct 2017 02:58:18 PM UTC, comment #25: 

I checked in version 3 of the patch here (http://hg.savannah.gnu.org/hgweb/octave/rev/196ea1ee99b8).  Closing report.

Rik <rik5>
Group administrator
Sat 14 Oct 2017 02:46:21 AM UTC, comment #24: 

The two bars are the same color, I assume.  That would be consistent with the third version, i.e., the equivalence of 'stacked' and 'grouped' when the input data is a vector as opposed to a matrix.  That's the most self-consistent in terms of setting individual properties after the initial plot.

Dan Sebald <sebald>
Fri 13 Oct 2017 06:28:41 AM UTC, comment #23: 

@Rik: in both cases, two bars at positions 1 and 2.

Marco Caliari <caliari>
Group Member
Thu 12 Oct 2017 06:35:34 PM UTC, comment #22: 

@Marco: Could you try the following under Matlab


h1 = bar ([3;4], 'stacked')
h2 = bar ([3,4], 'stacked')


Do the commands succeed?  Is the plot actually a single bar with two parts, or does it plot this as two bars?

Rik <rik5>
Group administrator
Sun 01 Oct 2017 06:14:41 AM UTC, comment #21: 

I actually convinced myself in post #19 that the third version of the changeset was correct.  :-)

Let's take a step back and think about consistency of the data and its interpretation (which I think version 3 is most self-consistent).  In version 4, there's this construct:


+  if (isvector (y))
+    if (group)
+      y = y(:);
+    else
+      y = y(:).';
+    endif
+  endif


I think any time the format of the stored data depends on an output property, such as stacked|grouped, it is precarious because somewhere the consistency is likely to fail.  [There should be a computer engineering term for such a thing if there isn't one already (I'm a different branch of engineering)... something like transmutable data, i.e., the data can represent one form of presentation or another form of presentation.]

Because all the properties can be altered after the original call to bar(), it's as if the processing of x and y has to be delayed to the final subroutine.  If after the fact, I change "barlayout" from a group to a stack, then we have to be certain that if y(:) is fed back through the bar() routine it's going to translate to y(:)', even with all the syntax variations.  In this case, I don't think it does that quite right.  Try this example:


octave:24> h = bar ([3; 4], 'grouped')
h = -6.1875
octave:25> get(h, 'ydata')
ans =

   3
   4

octave:26> set(h, 'barlayout', 'stacked')
octave:27> get(h, 'ydata')
ans =

   3
   4

octave:28> h = bar ([3; 4], 'stacked')
h =

  -6.1261
  -3.3075

octave:29> get(h, 'ydata')
ans =
{
  [1,1] =  3
  [2,1] =  4
}


The first stacked bar graph above is all purple (i.e., a plot of two stacks, one element per stack), the second is purple and yellow (i.e., a plot of one stack of two elements).  The original data in both cases was [3; 4], but it has reached a different result depending on the path we took.

This self-consistency is why I started thinking

bar ([3, 4], 'grouped')
bar ([3, 4], 'stacked')

should be thought of as the equivalent plot, i.e., in both cases the [3, 4] is the Y of bar(Y,...).

Dan Sebald <sebald>
Sun 01 Oct 2017 03:52:28 AM UTC, comment #20: 

I completely re-vamped the input validation.  See the attached bar.diff4.  It now correctly (???) processes


bar ([3, 4], 'stacked')


as well as stuff that even Matlab rejects like


bar (1, [3; 4], 'stacked')




(file #42030)

Rik <rik5>
Group administrator
Fri 29 Sep 2017 05:10:04 PM UTC, comment #19: 

Eh, maybe this is fine:


bar ([3 4 5], 'stacked')


If we think of the above as being stacked with three stacks and only a single sub-component per stack (rather than a single stack consisting of three sub-components), then it is the equivalent of


bar ([3 4 5], 'grouped')


thought of as three groups with a single sub-component per group.

Dan Sebald <sebald>
Fri 29 Sep 2017 04:53:19 PM UTC, comment #18: 

Has the uniqueness test, transposing at the end if necessary is good.  The one thing I'm still wondering about is how this should behave:


bar ([3 4], 'stacked')


To my way of thinking, that should produce a stacked bar graph.

Dan Sebald <sebald>
Fri 29 Sep 2017 03:48:59 PM UTC, comment #17: 

Okay, I think I've got it working now.  Attached is bar.diff3.  It is actually more forgiving then Matlab in that it tries both orientations, row or column, when attempting to match the length of the X vector.

(file #41982)

Rik <rik5>
Group administrator
Fri 29 Sep 2017 01:19:36 PM UTC, comment #16: 

In response to comments #4 and #13: Matlab R2017a


>> bar (1, [3; 4], 'stacked')
Error using bar (line 143)
X must be same length as Y.

>> h = bar (1:4)

h =

  Bar with properties:

    BarLayout: 'grouped'
     BarWidth: 0.8000
    FaceColor: 'flat'
    EdgeColor: [0 0 0]
    BaseValue: 0
        XData: [1 2 3 4]
        YData: [1 2 3 4]

  Use GET to show all properties

>> h2 = bar ([1 2], [1 2 3 4; 5 6 7 8], 'stacked')

h2 =

  1×4 Bar array:

    Bar    Bar    Bar    Bar



Marco Caliari <caliari>
Group Member
Thu 28 Sep 2017 11:17:51 PM UTC, comment #15: 

I agree that there is some question as to what is actually a grouped versus conventional bar graph in the vector case.

But the patch you created is close.  I think the issue is that you haven't covered every instance of rows/cols with the new mod.  My previous post had a "verbatim" issue, so I'm reposting.

The examples work, but it may have broken this case [similar to bar(1:10)]:


octave:4> h = bar ([1 2], [3 4])
error: bar: length of X and Y must be equal
error: called from
    __bar__ at line 123 column 5
    bar at line 118 column 16


Maybe this test:


  ngrp = rows (x);
  if (ngrp != rows (y))
    error ("%s: length of X and Y must be equal", func);
  endif


needs to be on cols(y) rather than rows(y).  Also, there is a rows() here after swapping x and y that should be double checked (should it be y = x.'?):


    if (isscalar (y) && ! isscalar (x))
      ## "y" is actually "width" argument
      y = x;
      x = [1:rows(y)]';
      idx = 2;


Dan Sebald <sebald>
Thu 28 Sep 2017 11:12:38 PM UTC, comment #14: 

The examples work, but it may have broken this case:


octave:4> h = bar ([1 2], [3 4])
error: bar: length of X and Y must be equal
error: called from
    __bar__ at line 123 column 5
    bar at line 118 column 16


Maybe this test:


  ngrp = rows (x);
  if (ngrp != rows (y))
    error ("%s: length of X and Y must be equal", func);
  endif
-verbatim_

needs to be on cols(y) rather than rows(y).  Also, there is a rows() here after swapping x and y that should be double checked (should it be y = x.'?):

+verbatim+
    if (isscalar (y) && ! isscalar (x))
      ## "y" is actually "width" argument
      y = x;
      x = [1:rows(y)]';
      idx = 2;


I'm out for a while, but the essence of the patch is good.

Dan Sebald <sebald>
Thu 28 Sep 2017 11:01:56 PM UTC, comment #13: 

Actually, my patch doesn't appear to work for


bar (1:10)


I think it would be useful to understand how many objects Matlab returns from bar.  Is it based on groups, or columns?


h = bar (1:4)
numel (h)
h2 = bar ([1 2], [1 2 3 4; 5 6 7 8], 'stacked')
numel (h2)



Rik <rik5>
Group administrator
Thu 28 Sep 2017 10:43:19 PM UTC, comment #12: 

Try the attached patch bar.diff2.

It

1) requires the first input to be numeric
2) requires any provided X values to be unique, but not necessarily in ascending order
3) makes all input Y vectors into column vectors which is what the remaining code expects.

It seems to work for me now on the various examples in this bug report.


(file #41955)

Rik <rik5>
Group administrator
Thu 28 Sep 2017 09:46:33 PM UTC, comment #11: 

Actually, this is a better illustration of my point:


octave:18> h = bar (1, [3 4], 'stacked');
octave:19> set(h(1), 'barlayout', 'grouped');
error: bar: length of X and Y must be equal
execution error in graphics callback function
octave:20>


Dan Sebald <sebald>
Thu 28 Sep 2017 09:34:55 PM UTC, comment #10: 

I can go along with that interpretation of bar(Y, width).

But there are a few things wrong with the patch.

The main thing is my original point that the data saved in the newly created property


    addproperty ("ydata", hg, "data", y(:, i));

    addlistener (hg, "xdata", @update_data);
    addlistener (hg, "ydata", @update_data);


cannot be conditioned on "stack", i.e., your line


     elseif (ischar (varargin{idx}) && strcmpi (varargin{idx}, "stacked"))
       group = false;
+      if (isvector (y))
+        y = y.';
+      endif


turns a column vector into a row vector.  It has to be the same format convention regardless of "stack" or "grouped" because after the initial "bar()" we should be allowed to change back and forth between the "barlayout" property.

As a consequence, there is this result with the patch:


octave:13> h = bar (1, [3 4], 'grouped')
error: bar: length of X and Y must be equal
error: called from
    __bar__ at line 120 column 5
    bar at line 118 column 16


Some other strange behavior (probably not unique to your patch):


octave:16> h = bar ('ae', [3 4]')
error: invalid value for array property "xdata"
error: called from
    __bar__>bars at line 323 column 7
    __bar__ at line 200 column 12
    bar at line 118 column 16


produces a plot.  Maybe we should also check to make sure the first argument is numeric and not a string.

Dan Sebald <sebald>
Thu 28 Sep 2017 08:57:28 PM UTC, comment #9: 

@Dan: Can you try my attached patch bar.diff?  This is an easy bug to fix by changing the input validation.  However, your cset doesn't handle this case


bar (1:3, 0.2)


where the calling form is


bar (Y, WIDTH)




(file #41952)

Rik <rik5>
Group administrator
Thu 28 Sep 2017 08:49:11 PM UTC, comment #8: 

The attached patch seems to do it by my way of thinking.  Let me know what you think.  Try:


h = bar (1, [3 4], 'stacked')


or (same result)


h = bar (1, [3; 4], 'stacked')


and follow up with


set(h(1), 'barlayout', 'grouped')


Note that for some reason the "h = bar()" is returning two handles, both of which seem to control the bar graph appearance.  Is that a separate bug?

(file #41951)

Dan Sebald <sebald>
Thu 28 Sep 2017 08:12:27 PM UTC, comment #7: 

I notice this error in the code:


  if (any (x(2:end) < x(1:end-1)))
    error ("%s: X vector values must be in ascending order", func);
  endif


which is not a requirement in the documentation.  X can have any order, but there can be no duplicate elements.

Dan Sebald <sebald>
Thu 28 Sep 2017 07:59:49 PM UTC, comment #6: 

Following up on my second point in the previous post, let's add the following example to the list of clarification:


bar ([3 4], 'stacked')


I think this should create a stacked bar chart, not a conventional bar chart.  Noting again that it doesn't matter if we think of


bar ([3 4], 'grouped')


as a group or a conventional bar chart because they are pretty much the same.

Dan Sebald <sebald>
Thu 28 Sep 2017 07:34:36 PM UTC, comment #5: 

I don't know.  Documentation is quite exactly clear.  However, a couple things come to mind.

  • It now seems to me that switching the property between grouped|stacked after the fact should not be a problem.  They share the same underlying data properties.


  • Regardless of how the vertical vector case is treated, it seems to me that in no scenario should the bar(numeric, numeric, ...) treat this as anything other than bar(X, Y, ...).  That is, in the following


octave:3> bar (1, [3, 4]', 'stacked')
x =  1
y =  1
args =
{
  [1,1] =

     3
     4

  [1,2] = stacked
}

X should be 1 and Y should be [3, 4]'; args{0} should not be [3 4]'.  Further processing would conclude whether that is a sane X/Y pair.

The bug-reporter does have a good point though.  If the documentation is taken literally, i.e., "Y being a vector" of no qualification horizontal or vertical, then there is no way of creating a grouped or stacked bar graph of just a single group (stack).  (The matrix scenario has no ambiguity.)  A single group of course is sort of indistinguishable from a plain bar graph, but a single stack is (well, I suppose if one includes barh, then it does sort of look like a barh plot, if one uses the extra Width property to achieve it).  I can see an argument for not doing the y = y(:)'.

Dan Sebald <sebald>
Thu 28 Sep 2017 04:00:21 PM UTC, comment #4: 

For compatibility, what does Matlab do for


bar (1, [3; 4], 'stacked')


If y is a row vector it is clear that each column represents a different group.  But in this case, Matlab might error out because two y-values have been specified which ostensibly should belong to different x-locations.


Rik <rik5>
Group administrator
Thu 28 Sep 2017 03:31:26 PM UTC, comment #3: 

In response to comment #2, the rest of the bar code depends on y being a column vector.  We can't just remove it or ordinary bar invocations will fail.

Just for grins, I commented those three lines out.  Here are the results


octave:1> bar ([1 2], [3; 4])  # Works, because Y is already a column vector
octave:1> bar ([1 2], [3, 4])  # Fails, because Y is a row vector
error: set: invalid number of arguments
error: called from
    __bar__>bars at line 340 column 7
    __bar__ at line 201 column 12
    bar at line 118 column 16



Rik <rik5>
Group administrator
Thu 28 Sep 2017 11:28:45 AM UTC, comment #2: 

Why is it not possible to simply remove the if block around line 47 in _bar_.m


if (isvector (y))
  y = y(:);
endif


?

Marco Caliari <caliari>
Group Member
Thu 28 Sep 2017 08:49:30 AM UTC, comment #1: 

Confirmed.  Printing out some info:


octave:2> bar ([1, 2], [3, 4; 5, 6], 'stacked')
x =

   1
   2

y =

   3   4
   5   6

args = {}(0x0)
args = {}(0x0)
args = {}(0x0)
args = {}(0x0)


looks fine, but not so here:


octave:3> bar (1, [3, 4]', 'stacked')
x =  1
y =  1
args =
{
  [1,1] =

     3
     4

  [1,2] = stacked
}

args =
{
  [1,1] =

     3
     4

  [1,2] = stacked
}

error: wrong type argument 'matrix'
error: set: argument 0 must be a property name
error: called from
    __bar__>bars at line 341 column 7
    __bar__ at line 199 column 12
    bar at line 118 column 16
octave:3>


In the code, because the number of rows (after transpose) of y doesn't equal that of x, the assumption is that the first argument is the Y (type "help bar" for first item on the list of valid formats).  As a consequence, the code than treats the second argument as a property of sorts, and that's what the error message pertains to, i.e., [3 4]' is not a character string no less a property name.

So, the existence of 'stacked' needs to be checked first prior to processing the data input variations.  It might be kind of tricky though, in the sense that the user might be able to switch 'barlayout' between stacked|grouped.  Have to think about what that means if the data is specified in a way that implicitly describes what could only be "stacked" layout.

Dan Sebald <sebald>
Thu 28 Sep 2017 06:36:29 AM UTC, original submission:  

If I call


bar ([1, 2], [3, 4; 5, 6], 'stacked')


I see 4 bars, stacked two by two. But if I call


bar (1, [3, 4], 'stacked')


I get an error message, while I would expect two stacked bars. The problem is around line 47 of _bar_.m. For some reason, the second argument y is transformed into a column.

Marco Caliari <caliari>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #52194:  strength@pract.m added by None (2KiB - application/octet-stream - how to get rid of that errors )
file #42030:  bar.diff4 added by rik5 (3KiB - application/octet-stream)
file #41982:  bar.diff3 added by rik5 (2KiB - application/octet-stream)
file #41955:  bar.diff2 added by rik5 (2KiB - application/octet-stream)
file #41952:  bar.diff added by rik5 (1KiB - 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 rik5 (Posted a comment)
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by caliari (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 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-11-03 None Attached File- Added strength@pract.m, #52194
    2017-10-14 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2017-10-01 rik5 Attached File- Added bar.diff4, #42030
    2017-09-29 rik5 Attached File- Added bar.diff3, #41982
    2017-09-28 rik5 Attached File- Added bar.diff2, #41955
        StatusIn Progress Patch Submitted
    2017-09-28 rik5 Attached File- Added bar.diff, #41952
        StatusConfirmed In Progress
    2017-09-28 sebald Attached File- Added octave-bar_graph_single_stack-djs2017sep28.patch, #41951
    2017-09-28 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code