bugGNU Octave - Bugs: bug #62420, inputParser fails due to...

 
 

bug #62420: inputParser fails due to interpreter changes in 7.1.0

Submitter:  Georg Wiora <gwiora>
Submitted:  Fri 06 May 2022 06:44:51 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  gwiora Open/Closed:  * Closed
Release:  * 7.1.0 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 05 Jul 2022 04:00:17 PM UTC, comment #8: 

I verified Octave works with minimal example from comment #5.

Marking bug as Fixed and closing report.

Rik <rik5>
Group administrator
Mon 06 Jun 2022 12:33:04 PM UTC, comment #7: 

Unfortunately, `nargout` doesn't work for built-in functions. So, we need to use a try-catch block to check for that error.
Additionally, we still need to call validation functions without output arguments again inside yet another try-catch block.

I checked in this change that will hopefully work correctly with Octave 7.x or newer and will hopefully work with all types of validation functions:
https://hg.savannah.gnu.org/hgweb/octave/rev/2c8ab613e805

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Mon 09 May 2022 06:36:57 AM UTC, comment #6: 

Hi Markus, your minimal example is perfect.

I think your suggestion of querying nargout would be superior to the try-catch-construct because it does not catch other errors and helps keeping code simple. The problem I see is that throwing the exception inside the validation function would not allow to display additional information in inputParser about the reasons.

Your suggested change of inputParser looks good so far. I would recommend testing the the error id of the exception to distinguish between "too few output values" and any implementation or runtime error inside the validation function. You may have a look on my comment #2 for this.


Georg Wiora <gwiora>
Fri 06 May 2022 04:30:11 PM UTC, comment #5: 

With respect to compatibility with Matlab:

>> a = validateattributes('s', {'char'}, {'nonempty'})
Error using validateattributes
Too many output arguments.

>> a = validateattributes(1, {'char'}, {'nonempty'})
Error using validateattributes
Too many output arguments.

>> validateattributes(1, {'char'}, {'nonempty'})
Expected input to be one of these types:

char

Instead its type was double.

>> validateattributes('s', {'char'}, {'nonempty'})


So, IIUC, it is correct that `validateattributes` doesn't return anything in Octave either.

The change probably needs to happen in the `inputParser` class.

I never used the `inputParser` before. So, I'm a bit at a loss. A minimal example that reproduces the error would really help.
After trying around a bit, I ended up with this that shows that error. (But I'm not sure if it is supposed to work like this at all.):

p = inputParser ();
p.addRequired ('rootPath', @(s) validateattributes (s, {'char'}, {'nonempty'}));

p.parse ('/usr/bin');


Instead of using a `try`-`catch`-block, would checking `nargout(val)>0` also work?
Would it be enough for the `inputParser` to directly let the "validation function" throw its error? Or would it be better to amend the error with additional information from the `inputParser`?

Something along these lines maybe?

diff -r 1ce1c4552d5b scripts/miscellaneous/inputParser.m
--- a/scripts/miscellaneous/inputParser.m        Thu May 05 20:47:09 2022 +0200
+++ b/scripts/miscellaneous/inputParser.m        Fri May 06 18:26:21 2022 +0200
@@ -543,9 +543,21 @@

     function validate_arg (this, name, val, in)

-      if (! val (in))
-        this.error (sprintf ("failed validation of %s with %s",
-                             toupper (name), func2str (val)));
+      if (nargout (val) > 0)
+        ok = val (in);
+        err = sprintf ('Checked with "%s"', func2str (val));
+      else
+        try
+          val (in);
+          ok = true;
+        catch exception
+          ok = false;
+          err = exception.message;
+        end_try_catch
+      endif
+      if (! ok)
+        this.error (sprintf ("input %s is invalid. %s",
+                             toupper (name), err));
       endif
       this.Results.(name) = in;


Markus Mützel <mmuetzel>
Group administrator
Fri 06 May 2022 12:50:31 PM UTC, comment #4: 

I try to get closer to the core of this error. I use a function dirPlus() from matlab fileexchange, that specifies validateattributes() as a validation function to input parser:

    addRequired(parser, 'rootPath', ...
                @(s) validateattributes(s, {'char'},{'nonempty'}));

Earlier versions of input parser specified that the validation function needs to return something on error and nothing if ok. This is why using validateattributes() was ok in this usage but now is not any more.

If there is a problem in the 7.1.0 it has been introduced to validateattributes() some versions ago. Nevertheless it should not produce any problem if validateattributes() just returns true in all cases? This would be an option to fix my troubles at least...

Georg Wiora <gwiora>
Fri 06 May 2022 12:02:56 PM UTC, comment #3: 

To avoid even more misunderstanding on my end, could you please show a minimal example that produces the error for you?

Markus Mützel <mmuetzel>
Group administrator
Fri 06 May 2022 07:45:06 AM UTC, comment #2: 

Sorry Markus, I had an outdated version of inputParser in my path. After removing the old version the problem essentially stays the same, just the code looks exactly as you quoted it.

It fails when executing val (in) because validatedattributes() that is provided as function handle in val is not returning a result if argument check is ok:

error: validateattributes: function called with too many outputs
error: called from
    validateattributes



After I rewrote the validate_arg() function like this it is behaving as expected:

    function validate_arg (this, name, val, in)

      try
        check = val(in);
      catch
        myerror = lasterror();
        if strcmpi(myerror.identifier,"Octave:invalid-fun-call")
          ## check is ok if no result
          check = true;
        else
          ## rethrow error
          disp(myerror());
          error(myerror.identifier,myerror.message);
        endif
      end_try_catch

      if (!check)
        this.error (sprintf ("failed validation of %s with %s",
                             toupper (name), func2str (val)));
      endif
      this.Results.(name) = in;

    endfunction


I can't tell if it would be better to modify validateattributes()

Georg Wiora <gwiora>
Fri 06 May 2022 07:07:05 AM UTC, comment #1: 

What does `which inputParser` return for you?
The `validate_arg` function in the inputParser.m file of Octave 7.1.0 doesn't contain the commands that are failing for you afaict.
It should be looking like this:
https://hg.savannah.gnu.org/hgweb/octave/file/1ce1c4552d5b/scripts/miscellaneous/inputParser.m#l544

    function validate_arg (this, name, val, in)

      if (! val (in))
        this.error (sprintf ("failed validation of %s with %s",
                             toupper (name), func2str (val)));
      endif
      this.Results.(name) = in;

    endfunction


Markus Mützel <mmuetzel>
Group administrator
Fri 06 May 2022 06:44:51 AM UTC, original submission:  

The validate_arg function as part inputParser class fails in octave 7.1.0 at the marked position while it worked up to 6.4

    function validate_arg (this, name, val, in)
            % Check validation result to be empty in case of the use of validateattributes function
            % An empty result will be interpreted as valid arguments
    -->        check = {val(in)};
            if (~isempty(check) && ~val(in))
              this.error (sprintf ("failed validation of %s with %s", toupper (name),func2str(val)));
            endif
            this.Results.(name) = in;
        endfunction


The reason is that val() is a function handle for validateattributes() that checks the validity of the in parameter and does only return a result if the test fails. If everything is ok it does not return a result. Due to changes in octave 7.1 interpreter this is now handled as an error (see bug #62418).

There may be more functions affected by this change.

Georg Wiora <gwiora>

 

(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

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by gwiora (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
    2022-07-05 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2022-06-27 mmuetzel Dependencies- bugs #62669 is dependent
    2022-06-06 mmuetzel StatusNeed Info Ready For Test
    2022-05-06 mmuetzel StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code