bugGNU Octave - Bugs: bug #58296, Improve description of warning...

 
 

bug #58296: Improve description of warning state

Submitted by:  Tasos Papastylianou <tpapastylianou>
Submitted on:  Sun 03 May 2020 12:23:27 PM UTC  
 
Category:  Documentation Severity:  2 - Minor
Priority:  3 - Low Item Group:  Documentation
Status:  Confirmed Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  6.0.90 Operating System:  Any

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)

Sat 09 May 2020 11:17:30 AM UTC, comment #13: 

As a partial step for #1, I went through warning_id.m, removed warning ids that are no longer used and added some documentation for warning ids that had none yet here:
https://hg.savannah.gnu.org/hgweb/octave/rev/50ced8ab6387

Markus Mützel <mmuetzel>
Project Member
Thu 07 May 2020 08:15:36 PM UTC, comment #12: 

I pushed a fix for item #2 here https://hg.savannah.gnu.org/hgweb/octave/rev/5e1f4060289a.

Only thing left is item #1 to make the documentation for warning() clearer.

Rik <rik5>
Project Administrator
Wed 06 May 2020 11:01:26 PM UTC, comment #11: 

I pushed a fix for pretty printing (issue #3) to the development branch here https://hg.savannah.gnu.org/hgweb/octave/rev/7eddf9608d61.

Rik <rik5>
Project Administrator
Wed 06 May 2020 10:07:18 PM UTC, comment #10: 

Addressing #2 is quite easy.  A question has come up, however.  How should "warning error" be treated?  Matlab has stopped documenting "error" as a legitamate state, however the interpreter still accepts it.  On the other hand, the interpreter disallows "warning error all".  Instead, you have to individually select which warnings you want to turn in to errors.

Any thoughts on how to proceed?  I implemented the default which was to have warning on|off|error turn everying to the specified state, but now the BIST tests won't pass.

Rik <rik5>
Project Administrator
Wed 06 May 2020 09:45:19 PM UTC, comment #9: 

Maybe one more:

3) 'warning query' should print a pretty printed list, same as 'warning' with no arguments, when nargout is zero.

Mike Miller <mtmiller>
Project Administrator
Wed 06 May 2020 09:35:20 PM UTC, comment #8: 

I see two things that I think are worth fixing here:

1) The documentation for warning().  It is a complicated function and it needs more explanation.

2) "warning on" should be equivalent to Matlab and equivalent to Octave's "warning on all"

Rik <rik5>
Project Administrator
Mon 04 May 2020 07:46:25 PM UTC, comment #7: 

As an example of my last comment, in version 3.6.4, this throws a warning:

>> PAGER = 'less'
warning: In recent versions of Octave, PAGER is a function instead
of a built-in variable.

By assigning to PAGER, you have created a variable that hides
the function PAGER. To remove the variable and restore the
function, type "clear PAGER"

That specific message was the only use of the "Octave:built-in-variable-assignment" warning ID.

Mike Miller <mtmiller>
Project Administrator
Mon 04 May 2020 07:38:52 PM UTC, comment #6: 

> throw a warning when one tries to set a warning id that does not pre-exist


This seems like a nice idea, but is unenforceable. Any string with a colon in it is a valid warning ID.

> I assumed that Octave:built-in-variable-assignment referred to masking a built in variable or function


That warning ID is not used by Octave anymore, it could be removed or marked as deprecated in the warning_ids list.

Once upon a time, this warning was only used to warn about specific built-in variable names, like 'PS1' or 'save_precision', that I guess used to be global variables but were changed into functions that you call like this to change the value: PS1(">> "). It was never a generic warning for any variable that shadows any function name.

Mike Miller <mtmiller>
Project Administrator
Mon 04 May 2020 06:23:35 PM UTC, comment #5: 

Dear Markus and Rik, thank you for taking the time to address this so diligently. I appreciate your arguments, and stand corrected.

I hope this doesn't come across like I'm trying to justify my entry (which I appreciate is annoying when it happens and all too common in such threads). However, I might as well complete the picture with some more issues I've spotted, perhaps you can shed some light there as well: best case scenario, there's still some actual issues there, which can then become proper bug
reports; worst case scenario, you'll give me even further good explanations why things are they way they are, and perhaps I can use this and contribute towards improving documentation. I would be happy to do so, time permitting.

1) Rik, thank you for pointing out that matlab behaves the same. You are absolutely right. I was thrown back when I tried my example, because matlab gave me this output:

warning( 'query', 'BohemianRhapsody:this-is-the-real-life' )
% Error using warning
% Unknown setting or incorrect message identifier
%'BohemianRhapsody:this-is-the-real-life'.

However, after seeing your example, it turns out that, you are indeed correct that matlab reports the state of unknown warning identifiers as 'status on' The problem above was not one of an unknown identifier as I initially thought. Rather, it simply turns out that octave allows dashes in the name, but matlab does not.

2) There still seem to exist at least a couple of visible differences with matlab, which are presumably bugs rather than intentional behaviour:

M: 'warning on' in matlab turns all warnings on.
O: 'warning on' in octave has no visible effect on currently off ids. They stay off.

Whereas 'warning on all' does the same in both systems

M: 'warning' and 'warning query all' have the same effect in matlab (i.e. resulting in a meaningful message)
O: 'warning' and 'warning query all' have different outputs in octave. The first gives a meaningful message, but the latter gives a struct (which is rather cryptic if it hasn't just been explained to you carefully by Markus and Rik!).

Regarding that last point, the special nature of the 'all' identifier is not obvious simply from the struct returned, unless one knows about it. A more appropriate terminal message would be needed, like matlab does it. At the very least, the nature of the 'all' struct should be explained in the documentation, whereas currently the nature of the 'all' id is only mentioned in passing. (as I said, I'd be happy to contribute if and when I find the time and figure out how
to do it).

3) I do believe that it would be advantageous to throw a warning (yes I'm aware of the irony) when one tries to set a warning id that does not pre-exist. Just because matlab allows silently it does not mean it shouldn't be there in octave. E.g. I can envisage a scenario where one sets Octave:shadowed-functions instead of Octave:shadowed-function all too easily.

4) You are absolutely correct about the shadowing warning. Ironically I had a different example in mind which was more elaborate, and in my attempt to simplify I chose an entirely inappropriate example. Having said that, I have just tested the example I did have in mind in matlab too, and this also fails to throw a warning. In case there is interest, the example is as follows:

# In the linux terminal, do:
mkdir a && echo "function myfun(); disp('a'); end" > ./a/myfun.m
mkdir b && echo "function myfun(); disp('b'); end" > ./b/myfun.m

# From matlab or octave, do:
addpath('a'); myfun
addpath('b'); myfun % no warning that myfun has been silently replaced

I would assume that warning of such conflicts having taken place (or indeed a warning when a variable name is used that previously belonged to a function) would be useful in octave, even if matlab doesn't warn about this either.

5) I assumed that Octave:built-in-variable-assignment referred to masking a built in variable or function (much like Rik's sin = 5 example), but this does not seem to be the case. What does this do then? It's not clear from the documentation. (and, in any case, this type of conflict would also be useful to flag with a warning).

Thanks again for your input, and sorry for the long ramble.

Tasos Papastylianou <tpapastylianou>
Sun 03 May 2020 05:23:12 PM UTC, comment #4: 

Re-posting my comment.

Case 1 : non-existent warnings

Octave doesn't have a full list of all warnings that can ever be produced because the list is dynamic.  You could open up an editor and write a new m-file which defines a new warning ID and Octave has no way of knowing about this.  Similarly, you could use addpath to add a whole directory of new functions, but in order to understand whether a new warning ID had been declared would require parsing every single function and storing the information away somewhere. 

Incidentally, Matlab behaves the same as Octave.  Try something like

warning ('query', 'MYID:test')

and it will report that the warning is enabled.

case 2 : turning on all warnings

This behavior is also Matlab compatible.  The warning ID 'all' is special.  When you ask for

warning query all

Octave or Matlab looks at the state of the 'all' ID (either 'on' or 'off') and then reports only the warnings which are in a different state.  The default warning state is 'on' and so Octave only reports on the warning IDs which are set to 'off'.  See Octave session below

octave:2> ws = warning ('query')
ws =

  1x15 struct array containing the fields:

    identifier
    state

octave:3> ws.identifier
ans = all
ans = Octave:array-as-logical
ans = Octave:array-to-scalar
ans = Octave:array-to-vector
ans = Octave:imag-to-real
ans = Octave:language-extension
ans = Octave:missing-semicolon
ans = Octave:neg-dim-as-zero
ans = Octave:resize-on-range-error
ans = Octave:separator-insert
ans = Octave:single-quote-string
ans = Octave:str-to-num
ans = Octave:mixed-string-concat
ans = Octave:variable-switch-label
ans = Octave:abbreviated-property-match

octave:4> ws.state
ans = on
ans = off
ans = off
ans = off
ans = off
ans = off
ans = off
ans = off
ans = off
ans = off
ans = off
ans = off
ans = off
ans = off
ans = off

When you execute

warning on all

then all warning IDs are in the same state as the special 'all' ID and so Octave only reports on the one ID.

Case 3 : shadowing warning

The shadowing warning is issued when a new function file shadows a built-in function, and not when a variable shadows a built-in function.  Matlab behaves similarly in that it lets you do this

sin = 5

To show this in Octave, execute this from a shell

mkdir ../+pkg
touch ../+pkg/sin.m

Next, start Octave, which will be fine.  But then execute

octave:1> addpath ..
warning: function /home/rik/wip/Projects_Mine/+pkg/sin.m shadows a built-in function
warning: function /home/rik/wip/Projects_Mine/+pkg/sin.m shadows a built-in function

Case 4 : only 14 warnings reported

This is the same issue as in Case 1 and Case 2.  Octave only reports warning IDs which differ from the current state of 'all'.

Rik <rik5>
Project Administrator
Sun 03 May 2020 05:14:51 PM UTC, comment #3: 

I think Rik crossed with my comment.
Resetting the fields to their updated values.

Markus Mützel <mmuetzel>
Project Member
Sun 03 May 2020 05:10:49 PM UTC, comment #2: 

Case 1 : non-existent warnings

Octave doesn't have a full list of all warnings that can ever be produced because the list is dynamic.  You could open up an editor and write a new m-file which defines a new warning ID and Octave has no way of knowing about this.  Similarly, you could use addpath to add a whole directory of new functions, but in order to understand whether a new warning ID had been declared would require parsing every single function and storing the information away somewhere. 

Incidentally, Matlab behaves the same as Octave.  Try something like

 in that it allows you to query non-existent warning IDs.  However, they take the opposite position about whether they are enabled and assume that the unknown ID is in the 'off' state.  Octave could change that default easily enough.

case 2

Rik <rik5>
Project Administrator
Sun 03 May 2020 04:59:57 PM UTC, comment #1: 

Maybe there is some confusion on how Octave's warning system works.
Essentially, a user can emit a warning with any id (following some rules though). So it is impossible to list all imaginable warnings with their respective state.
Instead, the default warning state applies to all warning ids unless the respective state is explicitly changed.

"warning on all" resets all warning ids to the default and sets the default to "on".

Maybe we could transform this report into a documentation bug.
It would be very much appreciated if you could help improve the documentation to better describe the behavior. Patches are always welcome.

The "Octave:shadowed-function" warning is also much more specific than one might expect. It is only emitted when the user changes to a directory that contains a function that shadows a core function or when such a directory is added to the search path. E.g. in a folder where you have write access:

fid = fopen ("clear.m", "w");
fprintf (fid, 'function clear(), disp ("test"); endfunction\n');
fclose (fid);
path (path);

Variables shadowing core functions (or even command-line functions) don't trigger the warning.
This could also be better documented.

Markus Mützel <mmuetzel>
Project Member
Sun 03 May 2020 12:23:27 PM UTC, original submission:  

The warning function behaves oddly.

Consider the following cases:

Case 1: Querying a non-existent warning reports it as being 'on'.

octave:1&gt; warning( 'query', 'BohemianRhapsody:this-is-the-real-life' )
ans =
  scalar structure containing the fields:
    identifier = BohemianRhapsody:this-is-the-real-life
    state = on

Case 2: Explicitly turning 'on' 'all' warnings breaks warning function.

octave:1&gt; warning query all
ans =
  1x14 struct array containing the fields:
    identifier
    state

octave:2&gt; warning on all
octave:3&gt; warning query all
ans =
  scalar structure containing the fields:
    identifier = all
    state = on

Case 3: Warnings regarding shadowing:

octave:1&gt; warning query Octave:shadowed-function
ans =
  scalar structure containing the fields:
    identifier = Octave:shadowed-function
    state = on

octave:2&gt; clear = 5
clear =  5

Case 4: Octave only reports the first 14 warnings explicitly. Furthermore, it says all are enabled, but then reports off.

octave:1&gt; warning
By default, warnings are enabled.

    off  Octave:array-as-logical
    off  Octave:array-to-scalar
    off  Octave:array-to-vector
    off  Octave:imag-to-real
    off  Octave:language-extension
    off  Octave:missing-semicolon
    off  Octave:neg-dim-as-zero
    off  Octave:resize-on-range-error
    off  Octave:separator-insert
    off  Octave:single-quote-string
    off  Octave:str-to-num
    off  Octave:mixed-string-concat
    off  Octave:variable-switch-label

What's going on?
Is this a regression bug?

PS. As an aside, the use of the keyword 'all' in the context of 'warning' is not properly documented in its helpstring / syntax description.

Tasos Papastylianou <tpapastylianou>

 

(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 rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by tpapastylianou (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 22 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-05-06 rik5 StatusNone => Confirmed
    2020-05-03 mmuetzel CategoryInterpreter => Documentation
        Severity3 - Normal => 2 - Minor
        Priority5 - Normal => 3 - Low
        Item GroupRegression => Documentation
        StatusWorks For Me => None
        Release5.2.0 => 6.0.90
        Operating SystemGNU/Linux => Any
        SummaryWeird warning behaviour - regression bug? => Improve description of warning state
    2020-05-03 rik5 Severity2 - Minor => 3 - Normal
        Priority3 - Low => 5 - Normal
        Item GroupDocumentation => Regression
        StatusNone => Works For Me
        Release6.0.90 => 5.2.0
        Operating SystemAny => GNU/Linux
        SummaryImprove description of warning state => Weird warning behaviour - regression bug?
    2020-05-03 mmuetzel Severity3 - Normal => 2 - Minor
        Priority5 - Normal => 3 - Low
        Item GroupRegression => Documentation
        Release5.2.0 => 6.0.90
        Operating SystemGNU/Linux => Any
        SummaryWeird warning behaviour - regression bug? => Improve description of warning state

    Back to the top


    Powered by Savane 3.5