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

 
 

bug #48038: fill errors/warnings

Submitted by:  Muhali <muhali>
Submitted on:  Fri 27 May 2016 09:13:23 AM UTC  
 
Category: PlottingSeverity: 3 - Normal
Priority: 5 - NormalItem Group: None
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: 4.0.2Operating System: GNU/Linux

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)

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>
Project 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:

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>
Project Member
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>
Project 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>
Project Member
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:

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>
Project Member
Fri 27 May 2016 02:08:20 PM UTC, comment #7:

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

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>
Project Member
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>
Project 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
what do you get?

and what do you get with:

Juan Pablo Carbajal <juanpi>
Project 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>
Project 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 File(s):
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 01 Jun 2016 11:53:06 PM UTCrik5StatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Tue 31 May 2016 02:52:52 PM UTCmmuetzelAttached File-=>Added input_check_for_patch_v2.patch, #37319
    Sat 28 May 2016 06:55:01 AM UTClachlanStatusNone=>Patch Submitted
    Fri 27 May 2016 05:32:04 PM UTCmmuetzelAttached File-=>Added input_check_for_patch.patch, #37291

    Back to the top


    Powered by Savane 3.1-cleanup1