Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

why are errors prepended with ???? #238

Closed
cbm755 opened this issue Oct 2, 2019 · 6 comments
Closed

why are errors prepended with ???? #238

cbm755 opened this issue Oct 2, 2019 · 6 comments

Comments

@cbm755
Copy link
Collaborator

cbm755 commented Oct 2, 2019

Doctests for errors look like:

% >> 2 + (1 + !))
% ??? parse error:

But why do we have this ???? It doesn't seem to be there in either Matlab or Octave. I traced it back to [1], the original handling of exceptions. And we've kept it that way ever since.

This could be changed before 1.0.0 (with some pain for projects using doctest).

[1] https://bitbucket.org/tgs/doctest-for-matlab/commits/4a322d4b85f773f303c0678ef0593e9483571e42

@cbm755
Copy link
Collaborator Author

cbm755 commented Oct 2, 2019

There are about half-dozen uses of this in Symbolic, here's a few:

     If conversion fails, you get an error:
          syms x
          double (x)
            ⊣ ??? ... can't convert expression ...
     If the matrix is singular, an error is raised:
          A = sym([1 2; 1 2]);
          inv(A)
            ⊣ ??? ... Matrix det == 0; not invertible...

Those are not great. I don't think the ??? tells user much. This would be better

     If the matrix is singular, an error is raised:
          A = sym([1 2; 1 2]);
          inv(A)
            ⊣ error: ... Matrix det == 0; not invertible...

For comparison, the actual error looks like:

>> inv(A)
error: Python exception: NonInvertibleMatrixError: Matrix det == 0; not invertible.
    occurred at line 3 of the Python code block:
    return x.inv(),
error: called from
    pycall_sympy__ at line 178 column 7
    inv at line 59 column 5

I feel we should consider dropping the ??? (or deprecating it).

@catch22
Copy link
Collaborator

catch22 commented Oct 2, 2019

I'm fine with replacing??? by error: (or dropping it in case it is already part of the exc.message).

@mtmiller
Copy link
Collaborator

mtmiller commented Oct 2, 2019

Worth mentioning that Texinfo has a @error command for this purpose, although we don't use that in Octave doc strings (yet). So for Texinfo syntax, this could be written as

@example
syms x
double (x)
@error{} ... can't convert expression ...
@end example

I see a few Octave doc blocks that use @result{} error: ..., I'm going to mail the maintainers list to ask for opinions on that vs @print vs @error.

@mtmiller
Copy link
Collaborator

mtmiller commented Oct 2, 2019

I'm going to mail the maintainers list to ask for opinions on that vs @print vs @error.

https://lists.gnu.org/archive/html/octave-maintainers/2019-10/msg00005.html

@mtmiller
Copy link
Collaborator

mtmiller commented Oct 3, 2019

I agree with dropping the ???, or deprecating and making it optional.

cbm755 added a commit that referenced this issue Oct 4, 2019
In most cases, prepend `error: ` like Octave does.  In special cases
(currently `parse error:` but maybe others later) we do not do this
prepending.

This is a partial fix for Issue #238.  But no changes yet on Matlab
and no deprecation of `???`.
@cbm755
Copy link
Collaborator Author

cbm755 commented Oct 4, 2019

There are differences about formatting to be accounted for.

  1. Octave usually, but not always, prepends error: to error messages. Octave hg tip:
>> 1 + (2 + !))
parse error:

  syntax error

>>> 1 + (2 + !))
              ^

>> nosuchfcn(42)
error: 'nosuchfcn' undefined near line 1 column 1
>> error('weee')
error: weee
>> 
  1. Matlab 2018b (2014b was similar) does not:
>> 1 + (2 + !))
 1 + (2 + !))
          |
Error: Invalid use of operator.
 
>> nosuchfcn(42)
Undefined function or variable 'nosuchfcn'.
 
>> error('weee')
weee
 
>> 
  1. exception object has a .message (see doctest_format_exception) which generally omits error: .

Octave

So I think we should prepend ex.message with error : except in the case when it already starts with parse error.

Maybe there are other special cases?

Matlab

Unsure, will do Octave change first...

@cbm755 cbm755 closed this as completed in e0cc599 Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants