bugGNU Octave - Bugs: bug #48038, fill errors/warnings

 
 

bug #48038: fill errors/warnings

Submitter:  Muhali <muhali>
Submitted:  Fri 27 May 2016 09:13:23 AM UTC
   
 
Category:  Plotting Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 4.0.2 Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 02 Jun 2016 05:03:46 PM UTC, comment #12: 

@Markus: I removed the check for an even number of input args in this cset (http://hg.savannah.gnu.org/hgweb/octave/rev/1955af16b42e).

Rik <rik5>
Group administrator
Thu 02 Jun 2016 10:52:39 AM UTC, comment #11: 

Thank you, Rik, for pushing the patch.

However, with your last change, the example from comment #7 fails with an error.
That syntax is un-documented in Matlab. But any combination of property-value-pairs with structs seem to work. Thus, the number of "surplus arguments" can be odd or even.

I guess that is a trade-off between allowing Matlab-compatible un-documented syntax and Octave erroring too late.

OTOH, an error is thrown after a patch is drawn with the following example anyway:

x = 1:10;
y1 = rand(1,10);
y2 = 2+rand(1,10);
props.LineStyle = ':';
fill ([x fliplr(x)], [y1 y2], 'r', props, {});


So why not consistently throw the error after drawing the patch and remove the check for an even number of "surplus arguments" completely?
Can the "unwind protect" be used to undo the changes if an error occured? If yes, should such a construct be avoided for some reason?
What do you think would be best?

Markus Mützel <mmuetzel>
Group administrator
Wed 01 Jun 2016 11:53:06 PM UTC, comment #10: 

I applied the patch to make the _patch_ input validation better here (http://hg.savannah.gnu.org/hgweb/octave/rev/4acfe6a99a9f).  I added a check on the number of input arguments which errors out before creating the patch if there are not an even number of PROPERTY/VALUE pairs.

Rik <rik5>
Group administrator
Tue 31 May 2016 02:52:52 PM UTC, comment #9: 

The attached patch also fixes the same issues while being much easier to read.

Sorry, for the noise.

(file #37319)

Markus Mützel <mmuetzel>
Group administrator
Fri 27 May 2016 05:32:04 PM UTC, comment #8: 

Attached, please find a patch fixing this issue.

Additionally, this allows for the Matlab compatible syntax:

patch ('LineStyle', ':');


which threw an error before.

For the examples in comment #0, a patch is drawn and an error is thrown. Matlab throws the error before drawing the patch.

(file #37291)

Markus Mützel <mmuetzel>
Group administrator
Fri 27 May 2016 02:08:20 PM UTC, comment #7: 

A related example that works in Matlab but fails in Octave is:

x = 1:10;
y1 = rand(1,10);
y2 = 2+rand(1,10);
props.LineStyle = ':';
fill ([x fliplr(x)], [y1 y2], 'r', props);

fill ([x fliplr(x)], [y1 y2], [1 0 0], props);


The error and warning messages are the same as given by Muhali.

The error is in _patch_.m not correctly parsing the input arguments when properties are set with a struct instead of with only property value pairs.

Muhali's second example should fail but produces a warning. The underlying issue is the same (_patch_.m messing up the input arguments).

I will try and see whether I can find a better way for _patch_.m to do the input check if this is OK for you.

Markus Mützel <mmuetzel>
Group administrator
Fri 27 May 2016 02:02:04 PM UTC, comment #6: 

well, it should at least, as in ML, return an error, so one would not have to deal with opengl warnings or an entirely unresponsive system.

Muhali <muhali>
Fri 27 May 2016 01:35:19 PM UTC, comment #5: 

Thanks, therefore there is nothing to change in Octave since it fails just like matlab.

This bug report can be closed.

Juan Pablo Carbajal <juanpi>
Group Member
Fri 27 May 2016 12:31:58 PM UTC, comment #4: 

The first command works the same as in octave (red square). The second gives the mentioned response, also when color is defined by a RGB triple.

Note that my original comment had erroneously 'fliplr' instead of 'fliplr(x)'.

Muhali <muhali>
Fri 27 May 2016 12:13:05 PM UTC, comment #3: 

Could you be more specific?

we you call

 fill([x fliplr], [y1 y2], "r")

what do you get?

and what do you get with:

fill([x fliplr], [y1 y2], "r", {})


Juan Pablo Carbajal <juanpi>
Group Member
Fri 27 May 2016 09:29:22 AM UTC, comment #2: 

Matlab says

Error using fill
Not enough input arguments.

Muhali <muhali>
Fri 27 May 2016 09:16:12 AM UTC, comment #1: 

Woith a singel extra argumnet in interprets as color/style. If you give two extra arguments it expects PROPERTY, VALUE pairs. So it say that property "r" is not a valid one.

Whether this is a bug or not needs a test in matlab. In which case it can be moved to "matlab compatibility"

Juan Pablo Carbajal <juanpi>
Group Member
Fri 27 May 2016 09:13:23 AM UTC, original submission:  

I am not if this is a bug. - With

x = 1:10;
y1 = rand(1,10);
y2 = 2+rand(1,10);
fill([x fliplr], [y1 y2], "r")

I get the expected filled figure. But when I - as I had done accidentally - add {} as an additional argument

fill([x fliplr], [y1 y2], "r", {})

I receive the error

error: set: unknown patch property r
error: called from
    _patch_ at line 182 column 7
    fill at line 116 column 20

I the color is specified as

fill([x fliplr], [y1 y2], [1 0 0], {})

one gets

warning: opengl_renderer: x/y/zdata should have the same dimensions. Not rendering.

each time the figure focus changes (leaving my system unresponsive within emacs).

Muhali <muhali>

 

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

Attach Files:
   
   
Comment:
   

Attached Files

 

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 lachlan (Updated the item)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by juanpi (Posted a comment)
  • -email is unavailable- added by muhali (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-06-01 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2016-05-31 mmuetzel Attached File- Added input_check_for_patch_v2.patch, #37319
    2016-05-28 lachlan StatusNone Patch Submitted
    2016-05-27 mmuetzel Attached File- Added input_check_for_patch.patch, #37291

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code