bugGNU Octave - Bugs: bug #29986, Minor: calling plot with no...

 
 

bug #29986: Minor: calling plot with no arguments results in errors (instead of just usage)

Submitted by:  Jake <epy>
Submitted on:  Sat 29 May 2010 12:41:44 AM UTC  
 
Category: LibrariesSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: David Bateman <dbateman>
Originator Name: Open/Closed: Closed
Release: 3.2.4Operating 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 24 Jun 2010 10:08:27 PM UTC, comment #8:

The problem with print_usage (caller) in _plt_ is that it doesn't give the same result as print_usage () in the calling function. You get a traceback message, even when the calling function is executed at the top level. Results like that can be confusing, which is I think part of the reason for this bug report in the first place.

John W. Eaton <jwe>
Project Administrator
Thu 24 Jun 2010 09:43:55 PM UTC, comment #7:

Given that 'print_usage' accepts an optional function name as input can't you just do something like

print_usage (caller);

in '__plt__' instead of

print_usage ();

?

Søren

Søren Hauberg <hauberg>
Thu 24 Jun 2010 07:23:57 PM UTC, comment #6:

I think JWE's approach is probably the way to go, although there was still a bit more work to be done. All of the plot commands accept the same sorts of arguments so it had been convenient to have the error message in one place. When we rely on print_usage back in the 6 .m files then we also need to update their documentation.

For example, semilogx says the calling convention is just 'semilogx (ARGS)' whereas it really accepts around 6 different calling conventions.

I have updated the documentation and checked in a change here.

http://hg.savannah.gnu.org/hgweb/octave/rev/390d93e20531

Rik <rik5>
Project Administrator
Wed 23 Jun 2010 08:33:15 PM UTC, comment #5:

I think I was trying to be too clever. Your solution is cleaner even if it is more intrusive

D.

David Bateman <dbateman>
Project MemberIn charge of this item.
Wed 23 Jun 2010 04:13:57 PM UTC, comment #4:

How about detecting the invalid number of arguments case in the plot, polar, loglog, semilogx, and semilogy functions? Patch attached.

(file #20798)

John W. Eaton <jwe>
Project Administrator
Wed 23 Jun 2010 03:29:51 PM UTC, comment #3:

What Rik suggests will help with the quadl function, but it won't help for the plot function... The way the print_usage function works is that it calls dbstack, then counts the depth of the function stack. From that it can figure out if the offending function was called from the top-level or not... We could change the error line in _plt_.m to

evalin ("caller", "print_usage()")

but then the caller stack would be

plot
_plt_
plot
print_usage

and the print_usage function would think that plot wasn't called from the toplevel.. The only way I see to really fix this is to have _plt_ return with a flag that the call was invalid that have the six functions that called _plt_ then call print_usage(), though this is ugly as it means more code than is strictly needed just to get rid of a trivial error message. Attached is a patch that would make this change. More code is not a great thing and so I hesitate to make such change, but will with a little convincing

D.

(file #20796)

David Bateman <dbateman>
Project MemberIn charge of this item.
Fri 11 Jun 2010 10:03:56 PM UTC, comment #2:

In general, functions should thoroughly check their input arguments before proceeding with calculation in order to avoid Garbage In, Garbage Out.

Octave over time has grown to have 830 m-files and, unfortunately, not all of them have been coded with input validation checks.

The case of plot.m is a little more obscure so I will put it aside for the moment.

For quadl, however, the fix is very simple. Take a look at legendre.m from the Mercurial repository where I recently added better input validation. These lines should be at the beginning of every m-file.

if (nargin < 2 || nargin > 3)
print_usage ();
endif

where the exact number of acceptable arguments varies between functions. I might slowly work through the m-files some time adding validation, but if you have patches for any favorites like quadl I can apply them now. Just attach to this bug report.

Rik <rik5>
Project Administrator
Thu 03 Jun 2010 10:50:02 PM UTC, comment #1:

Here's another function that dumps out errors instead of usage:

octave:2> quadl
error: `a' undefined near line 66 column 12
error: evaluating argument list element number 1
error: evaluating argument list element number 1
error: called from:
error: /usr/local/share/octave/3.2.4/m/general/quadl.m at line 66, column 3

Jake <epy>
Sat 29 May 2010 12:41:44 AM UTC, original submission:

octave:1> plot
usage: plot (y)
plot (x, y, ...)
plot (x, y, fmt, ...) plot (x, y, property, value, ...)
error: /usr/local/share/octave/3.2.4/m/plot/__plt__.m at line 116, column 5
error: /usr/local/share/octave/3.2.4/m/plot/plot.m at line 186, column 5
octave:1>

I looked at the m files in question and I'm scratching my head because I don't know why those lines would throw errors.

The line in plot.m is "axes (oldh)", where oldh is set to gca(). "axes(gca())" doesn't seem to throw an error. The line in _plt_.m is "usage(msg)", where msg is the usage message you see above.

The only thing I noticed right off-hand is that line 114 of _plt_.m "msg = sprintf ("%s %s (x, y, fmt, ...)", msg, caller);" is missing the trailing newline character. Of course that wouldn't make it throw an error, but that's why the last usage line is on the same line as previous.

Anyway, I'm sure it'll be super obvious to someone else as to why the errors are appearing and not just the usage message.

Jake <epy>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #20798:  diffs added by jwe (8KiB - application/octet-stream)
file #20796:  patch.plot added by dbateman (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by hauberg (Posted a comment)
  • -unavailable- added by jwe (Updated the item)
  • -unavailable- added by dbateman (Updated the item)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by epy (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
    Thu 24 Jun 2010 07:23:57 PM UTCrik5StatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Wed 23 Jun 2010 04:13:57 PM UTCjweAttached File-=>Added diffs, #20798
    Wed 23 Jun 2010 03:29:51 PM UTCdbatemanAttached File-=>Added patch.plot, #20796
      StatusNone=>Patch Submitted
      Assigned toNone=>dbateman

    Back to the top


    Powered by Savane 3.1-cleanup1