bugGNU Octave - Bugs: bug #32700, nargout(fn) Matlab compatibility

 
 

bug #32700: nargout(fn) Matlab compatibility

Submitter:  None
Submitted:  Mon 07 Mar 2011 11:35:27 AM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  jordigh
Originator Name:  Iain Murray Originator Email:  -email is unavailable-
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

Wed 04 Jan 2012 12:49:40 PM UTC, comment #8: 

The merge commit is showing which files have changed between the time you submitted your patch and the current development status. It's fine.

Closing this bug...

Jordi GutiƩrrez Hermoso <jordigh>
Group Member
Wed 04 Jan 2012 10:58:28 AM UTC, comment #7: 

The current Hg development version builds for me and nargin and nargout now work for me there. Thanks Jordi! (Perhaps you can close this bug.)

I don't understand why:
http://hg.savannah.gnu.org/hgweb/octave/rev/a74954d70c18
with the ChangeLog entry "merge in Iain Murray's changes" mentions so many files that have nothing to do with the patch. But I don't really understand Hg deeply either. As long as you're happy that nothing has been wrongly updated, I'm happy.

Iain Murray <imurray>
Mon 02 Jan 2012 06:17:57 PM UTC, comment #6: 

Sorry it has taken a while to get back to you. Next time this happens, ping us ever so once in a while, maybe once a month. We are not deliberately ignoring you. Like all volunteer-driven projects, we sometimes express more interest in some issues than others, and our interest wavers. Just be patient until we get back to you, but not too patient. Wait a month, not nine.

The timing right now is a bit off, since we're focussing on a new stable release, probably to be released this week, so we are trying to only apply patches that fix critical bugs (after all, we want this to be stable). I think your patch will have to wait until the next release.

I have therefore applied it to the development branch, and merged it in with current development:

http://hg.savannah.gnu.org/hgweb/octave/graph/a74954d70c18

It will be some time before this patch sees a stable release. We're trying to make more frequent stable releases. Four months is the release cycle I remember hearing.

Jordi GutiƩrrez Hermoso <jordigh>
Group Member
Mon 02 Jan 2012 12:39:16 PM UTC, comment #5: 

Original submitter here. Is there anything further I can do to get the patch for this bug/feature-enhancement to be applied? After initially submitting a diff, it was requested that I submit an Hg changeset. I learned how to use Hg just to fulfill this request(!), but the fix still wasn't applied.

I do understand you're all busy. But I'd like to gently point out that this experience hasn't made me feel particularly motivated to submit any other patches.

Iain Murray <imurray>
Mon 14 Nov 2011 10:52:56 AM UTC, comment #4: 

Please can I put a plea in for this patch to be applied to the octave sources?

This fix makes the business of implementing subsref() and subsasgn() methods in octave classes hugely cleaner (in particular if you wish to make the "." index type work for class methods).

I've just applied the second patch provided to a copy of the  3.4.3 sources and it appears to do the trick (once I'd made a couple of trivial changes to the patchfile).

Is there anything I can usefully do to assist this patch getting integrated into octave?

Patrick Gosling <jpmg>
Thu 10 Mar 2011 09:29:22 PM UTC, comment #3: 

(Iain here again.) Thanks for the pointers. I've attached (hopefully correctly) an hg changeset including a very similar fix for `nargin', with exactly the same motivation.

Personally I would wait for someone to express a need before making intrusive changes to Octave to make nargin/nargout work on builtins. Certainly it won't come up in the use-case I was interested in. Also as I stated before, Matlab has functions (well, at least `histc') which don't work in nargin/nargout.


Here is a demo of the new behavior of nargin:

octave:1> nargin(@()1)
ans = 0
octave:2> nargin(@(x)1)
ans =  1
octave:3> nargin(@(x,y)1)
ans =  2
octave:4> nargin(@(x,y,varargin)1)
ans = -3
octave:5> nargin(@(varargin)1)
ans = -1
octave:6> nargin(inline('1'))
ans = 0
octave:7> nargin(inline('1','x'))
ans =  1
octave:8> nargin(inline('1','x','y'))
ans =  2
octave:9> nargin(inline('1','x','y','varargin'))
ans = -3
octave:10> nargin(inline('1','varargin'))
ans = -1
octave:11> nargin(@histc)
ans =  3
octave:12> nargin(@sin)
error: nargin: nargin information not available for builtin functions
octave:13> (@(x,y,z) nargin)(1,1,1)
ans =  3
octave:14> (@(x,y,z,varargin) nargin)(1,1,1)
ans =  3
octave:15> (@(x,y,z,varargin) nargin)(1,1,1,1)
ans =  4


(file #22891)

Anonymous
Thu 10 Mar 2011 05:22:33 PM UTC, comment #2: 

Thanks for submitting a diff.

Before it can be applied, it should include a ChangeLog entry for the changes.

It would also be helpful if you created a proper changeset using "hg export" so that we don't have to add your name and patch description for you.

Also, please use C++ style comments in Octave code rather than C style comments.

There are some guidelines for contributors here:

http://www.gnu.org/software/octave/doc/interpreter/Contributing-Guidelines.html#Contributing-Guidelines

There is currently no way to get the number of output arguments from a builtin function.  To do that would require adding more arguments to the DEFUN macro.  Before making a change like that, I think it would be good to discuss it on the maintainers mailing list.

John W. Eaton <jwe>
Group administrator
Thu 10 Mar 2011 04:33:02 PM UTC, comment #1: 

I (the original submitter) have now attached a patch (the result of `hg diff'), which fixes a bug as a side-effect (sorry) of extending the functionality of nargout to match Matlab.

I think I originally misclassified this bug as "wishlist". Because I think the fact that `nargout' returns -1 in this context:

% Set testcase.m to contain the following and run it:
function testcase()
nargout('bar')
function [a,b,c] = bar()
[a,b,c] = deal(1,2,3);

is a bug. The patch fixes this and 3 is now returned, as it is in Matlab.

Other functionality introduced by the patch (all of which matches Matlab's behavior as far as I can tell):

  • nargout can be given a function handle as well as a string
  • nargout(fcn) works in the top level
  • inline functions always return 1
  • anonymous functions always return -1
  • functions with varargout optionally preceded by named outputs return -(length_output_list_including_the_varargout)


The final bullet means that `nargout(@more_out)' returns -3 for:

function [x,y,varargout] = more_out()
x = 1;
y = 2;


Even with the patch, nargout(@sin) does not work: I couldn't immediately work out how to get the number of outputs of builtin functions. Can anyone help? If not, this probably isn't a large cause for concern. Matlab's nargout doesn't work on all functions:

>> nargout(@histc) % works in Octave with my patch, because histc is an m-file
??? Error using ==> nargout
histc does not know how to answer nargin/nargout.

Although `nargout(@sin)' does work in Matlab.

The patch updates the documentation to take into account all of the above.

I've just noticed that, of course, `nargin' needs similar modifications to be Matlab compatible. Shall I try to do a patch for that too?


(file #22878)

Anonymous
Mon 07 Mar 2011 11:35:27 AM UTC, original submission:  

In Matlab the nargout function can take a function handle or name and will return the number of output arguments that it takes. The number is negative for variable numbers of outputs and (undocumented) -1 if the number is variable "[varargout]".

In Octave (according to the documentation) "At the top level, `nargout' is undefined." The only functions Octave will report nargout for are those within the same m-file (and Octave 3.2.3 reports -1 for those regardless). Support for function handles has not been implemented at all. Example output in Matlab and Octave are given below.

Use case: I ran into this compatibility wishlist item while trying to use a routine for robustly timing functions:
    http://www.mathworks.com/matlabcentral/fileexchange/18798
    (Motivated here: http://blogs.mathworks.com/steve/2008/02/29/timing-code-in-matlab/ )
This routine works in Octave if its second input argument is specified, as then nargout isn't called. Otherwise it crashes.





>> fn = @() deal(1,2,3)
fn =
    @()deal(1,2,3)
>> nargout(fn)
ans =
    -1






octave:1> fn = @() deal(1,2,3)
fn =

@() deal (1, 2, 3)

octave:2> nargout(fn)
error: octave_base_value::convert_to_str_internal (): wrong type argument `function handle'
error: nargout: expecting string as first argument






% Set foo.m to contain the following:
function foo()
nargout('bar')
function [a,b,c] = bar()
[a,b,c] = deal(1,2,3);


Running foo in Matlab prints out '3', whereas Octave prints out '-1'.




I'm not familiary enough with Octave internals to immediately write a patch. I just got as far as finding nargout implemented at the line beginning "DEFUN (nargout," in ov-usr-fcn.cc

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #22891:  nargin_nargout.diff added by None (7KiB - text/x-patch)
file #22878:  nargout.patch added by None (4KiB - text/x-patch - Proposed patch to fix and extend nargout)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jordigh (Posted a comment)
  • -email is unavailable- added by imurray (Posted a comment)
  • -email is unavailable- added by jpmg (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by None (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
    2012-01-04 jordigh StatusNone Fixed
        Assigned toNone jordigh
        Open/ClosedOpen Closed
    2011-03-10 None Attached File- Added nargin_nargout.diff, #22891
    2011-03-10 None Attached File- Added nargout.patch, #22878

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code