bugGNU Octave - Bugs: bug #50752, inputParser Parameter key are...

 
 

bug #50752: inputParser Parameter key are parsed as Optional

Submitter:  John W. Eaton <jwe>
Submitted:  Fri 07 Apr 2017 04:19:09 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  jwe Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 10 Apr 2017 02:45:53 PM UTC, comment #8: 

I got someone to test those so I pushed the following tests

http://hg.savannah.gnu.org/hgweb/octave/rev/5a3c3ff03167

Carnë Draug <carandraug>
Group Member
Mon 10 Apr 2017 02:10:30 PM UTC, comment #7: 

This error message is just wrong.  It looks like a Matlab bug so I don't think we will replicate it:


p = inputParser ();
p.addOptional ('op1', 'val1');
p.addParameter ('line', 'circle');
p.parse ('line');
p.Results ()

The argument 'line' is a string scalar or character vector and does not match any parameter names. It failed validation for the argument 'op1'.


Can you run one last test, please:


p = inputParser ();
p.addOptional ('op1', 'val1');
p.addParameter ('line', 'circle', @ischar);
p.parse ('line', 'line', 89);
p.Results ()


Carnë Draug <carandraug>
Group Member
Sun 09 Apr 2017 08:31:59 PM UTC, comment #6: 

Here the output for the 3 snippets:


% Does this errors about missing value for parameter line?
p = inputParser ();
p.addOptional ('op1', 'val1');
p.addParameter ('line', 'circle');
p.parse ('line');
p.Results ()

The argument 'line' is a string scalar or character vector and does not match any parameter names. It failed validation for the argument 'op1'.



 % Does this error about invalid argument for line? Or does it
% backtracks and retests it as values for op1 and op2?
p = inputParser ();
p.addOptional ('op1', 'val1');
p.addOptional ('op2', 'val2');
p.addParameter ('line', 'circle', @ischar);
p.parse ('line', 89);
p.Results ()

The value of 'line' is invalid. It must satisfy the function: ischar.



 % If there's enough arguments to fill the positional options
% and param/key, does it do it?
p = inputParser ();
p.addOptional ('op1', 'val1');
p.addOptional ('op2', 'val2');
p.addParameter ('line', 'tree');
p.parse ('line', 'circle', 'line', 'rectangle');
p.Results ()

ans = struct with fields:
    line: 'rectangle'
     op1: 'val1'
     op2: 'val2'


A.R. Burgers <arb>
Sun 09 Apr 2017 04:54:31 PM UTC, comment #5: 

Well, Matlab compatibility seems to make it impossible to have
positional options whose value is a parameter key.  I pushed this
change which adds those tests and also works for Switch options:

http://hg.savannah.gnu.org/hgweb/octave/rev/f1bf2590272a

Can someone with Matlab tell us how it behaves in the following cases?

+verbatim-+
%% Does this errors about missing value for parameter line?
p = inputParser ();
p.addOptional ('op1', 'val1');
p.addParameter ('line', 'circle');
p.parse ('line');
p.Results ()
-verbatim-


%% Does this error about invalid argument for line? Or does it
%% backtracks and retests it as values for op1 and op2?
p = inputParser ();
p.addOptional ('op1', 'val1');
p.addOptional ('op2', 'val2');
p.addParameter ('line', 'circle', @ischar);
p.parse ('line', 89);
p.Results ()




%% If there's enough arguments to fill the positional options and
param/key, does it do it?
p = inputParser ();
p.addOptional ('op1', 'val1');
p.addOptional ('op2', 'val2');
p.addParameter ('line', 'tree');
p.parse ('line', 'circle', 'line', 'rectangle');
p.Results ()


Carnë Draug <carandraug>
Group Member
Sat 08 Apr 2017 08:33:13 AM UTC, comment #4: 

I get this:


>> version

ans = 9.1.0.441655 (R2016b)

>> p = inputParser ();
p.addOptional ('op1', 'val1');
p.addOptional ('op2', 'val2');
p.addParameter ('line', 'tree');
p.parse ('line', 'circle');
p.Results ()

ans =

  struct with fields:

    line: 'circle'
     op1: 'val1'
     op2: 'val2'


A.R. Burgers <arb>
Fri 07 Apr 2017 06:38:59 PM UTC, comment #3: 

Based on the comment for the test, I assumed that Matlab behaved as the test expected.  For your example, that would mean setting line to "circle" and op1 and op2 to their default values.  Could someone test this in Matlab?

John W. Eaton <jwe>
Group administrator
Fri 07 Apr 2017 05:53:34 PM UTC, comment #2: 


> It seems reasonable to me to stop processing optional parameters as soon as a name/value pair is recognized. Is there some reason not to do that?


The name of a parameter can also be a valid value for a positional option.  Since positional options must come first, if a value is a valid positional option, then it should be treated as such instead as key/value pair.  Consider this case:


p = inputParser ();
p.addOptional ("op1", "val1")
p.addOptional ("op2", "val2")
p.addParameter ("line", "tree")
p.parse ("line", "circle")
p.Results ()


Should op1 and op2 should be set to line and circle respectively, and line be set to the default value of tree?  Or should op1 and op2 be set to their default values and line be set to circle?

I think the current behaviour is the most logical since positional options should be all be processed before trying to match key value parameters.  But I guess we need to do whatever Matlab does.  What does Matlab do?

Carnë Draug <carandraug>
Group Member
Fri 07 Apr 2017 04:25:52 PM UTC, comment #1: 

I checked in the following changeset:

  http://hg.savannah.gnu.org/hgweb/octave/rev/7b594fcfa32b

It seems reasonable to me to stop processing optional parameters as soon as a name/value pair is recognized.  Is there some reason not to do that?

John W. Eaton <jwe>
Group administrator
Fri 07 Apr 2017 04:19:09 PM UTC, original submission:  

Creating this bug report so that the following test in inputParser.m can have a bug report number:


%!xtest
%! p = inputParser;
%! p.addOptional ("op1", "val");
%! p.addParameter ("line", "tree");
%! p.parse ("line", "circle");
%! assert (p.Results, struct ("op1", "val", "line", "circle"));


John W. Eaton <jwe>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by arb (Posted a comment)
  • -email is unavailable- added by jwe (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-04-10 carandraug StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2017-04-07 carandraug Summarytest failure for inputParser inputParser Parameter key are parsed as Optional
    2017-04-07 jwe StatusNone Ready For Test

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code