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)

Submitter:  Jake <epy>
Submitted:  Sat 29 May 2010 12:41:44 AM UTC
   
 
Category:  Libraries Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  dbateman
Originator Name:  Open/Closed:  * Closed
Release:  * 3.2.4 Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

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>
Group 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>
Group 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>
Group Member
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>
Group 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>
Group Member
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>
Group 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 Files:
   
   
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
  • -email is unavailable- added by hauberg (Posted a comment)
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by dbateman (Updated the item)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by epy (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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-06-24 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2010-06-23 jwe Attached File- Added diffs, #20798
    2010-06-23 dbateman Attached File- Added patch.plot, #20796
        StatusNone Patch Submitted
        Assigned toNone dbateman

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code