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

Test documentation for ExceptionGroup #104150

Open
stevewanash opened this issue May 4, 2023 · 11 comments
Open

Test documentation for ExceptionGroup #104150

stevewanash opened this issue May 4, 2023 · 11 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@stevewanash
Copy link

stevewanash commented May 4, 2023

The new feature ExceptionGroup catches several exceptions at a time. It's helped me write functions that detect more than one error at a time, depending on the input. While helpful, the feature has made documentation, especially using doctest, nearly impossible. To test for exceptions, more so for Traceback stacks, before python version 3.11, the documentation only needed the traceback statement and the name of the error to test the function. Citing your own documentation it looked something like this:

"""
Traceback (most recent call last):
    ...
ValueError: detail
"""

The elipses replaces the detail within the traceback stack, and could be omitted all together, to look like this:

"""
Traceback (most recent call last):
ValueError: detail
"""

However, with ExceptionGroups now, The format of the exception doesn't allow this. An example output of an ExceptionGroup would look like this:

  + Exception Group Traceback (most recent call last):
  |   File "<stdin>", line 1, in <module>
x_divided                            
  |     raise ExceptionGroup("There were errors", errors)
  | ExceptionGroup: There were errors (3 sub-exceptions)
  +-+---------------- 1 ----------------
    | TypeError: matrix must be a matrix (list of lists) of integers/floats
    +---------------- 2 ----------------
    | TypeError: Each row of the matrix must have the same size
    +---------------- 3 ----------------
    | ZeroDivisionError: division by zero
    +------------------------------------ 

The added "+" and "|", which I assume were added for indentation purposes, make it difficult to write up test documentation for the code. Ideally, without omitting anything in the traceback stack, test documentation would look like this:

"""
Test for more than one error
>>> s([[2, 4], [6, 6, "f"]], 0)
  + Exception Group Traceback (most recent call last):
  |   File "<stdin>", line 1, in <module>
x_divided                            
  |     raise ExceptionGroup("There were errors", errors)
  | ExceptionGroup: There were errors (3 sub-exceptions)
  +-+---------------- 1 ----------------
    | TypeError: matrix must be a matrix (list of lists) of integers/floats
    +---------------- 2 ----------------
    | TypeError: Each row of the matrix must have the same size
    +---------------- 3 ----------------
    | ZeroDivisionError: division by zero
    +------------------------------------
"""

This however, would result in a failed test for the function S. The output would look like this:

Failed example:
    s([[2, 4], [6, 6, "f"]], 0)
Exception raised:
      + Exception Group Traceback (most recent call last):
      |   File "C:\Users\pc\AppData\Local\Programs\Python\Python311\Lib\doctest.py", line 1351, in __run
      |     exec(compile(example.source, filename, "single",
      |   File "<doctest 2-matrix_divided.txt[5]>", line 1, in <module>
      |     s([[2, 4], [6, 6, "f"]], 0)
      |   File "C:\Users\pc\My Github code\alx-higher_level_programming\0x07-python-test_driven_development\2-matrix_divided.py", line 35, in matrix_divided
      |     raise ExceptionGroup("There were errors", errors)
      | ExceptionGroup: There were errors (3 sub-exceptions)
      +-+---------------- 1 ----------------
        | TypeError: matrix must be a matrix (list of lists) of integers/floats
        +---------------- 2 ----------------
        | TypeError: Each row of the matrix must have the same size
        +---------------- 3 ----------------
        | ZeroDivisionError: division by zero
        +------------------------------------

I assume the reason for the test failing is the difference in the Traceback stack, but clearly from the first example, and all your previous documentation, whatever is within the traceback stack is usually ignored by python and can be replaced by an ellipses(...) or omitted entirely. This isn't the case here as removing the information within the Traceback stack, as shown below, would give the same output, a failed test:

"""
Test for more than one error
>>> s([[2, 4], [6, 6, "f"]], 0)
  + Exception Group Traceback (most recent call last):
  |       ...
  | ExceptionGroup: There were errors (3 sub-exceptions)
  +-+---------------- 1 ----------------
    | TypeError: matrix must be a matrix (list of lists) of integers/floats
    +---------------- 2 ----------------
    | TypeError: Each row of the matrix must have the same size
    +---------------- 3 ----------------
    | ZeroDivisionError: division by zero
    +------------------------------------
"""

Similarly, if I decided to ignore the "|" and "+" symbols at the beginning, and the "-" separating the errors, I would still get the same result, a failed test.

"""
Test for more than one error
>>> s([[2, 4], [6, 6, "f"]], 0)
Exception Group Traceback (most recent call last):
    ...
ExceptionGroup: There were errors (3 sub-exceptions)
      ...
  TypeError: matrix must be a matrix (list of lists) of integers/floats
      ...
  TypeError: Each row of the matrix must have the same size
      ...
  ZeroDivisionError: division by zero

"""

None of them work, and will all result in a failed test, even when I copy the output of the interpreter straight to the test documentation. This, I suppose, is due to the extra formatting when the ExceptionGroup is caught, that is the "+", "|", and "-" symbols. Doctest immediately assumes the output it gets is different from that of the test documentation, owing to the extra formatting and no provisions on how to handle it. Please fix the issue, or formulate some rules on how to write documentation when it comes to ExceptionGroups. Further clarification on the issue can be found in the link below.
https://stackoverflow.com/questions/76167508/how-do-i-write-test-documentation-for-exception-groups

I used VScode running on python version 3.11. I ran the doctest on the command prompt terminal within VScode, on a windows operating system.

@stevewanash stevewanash added the type-bug An unexpected behavior, bug, or error label May 4, 2023
@AlexWaygood AlexWaygood added the stdlib Python modules in the Lib dir label May 4, 2023
@terryjreedy
Copy link
Member

@iritkatriel issue re exception group

@iritkatriel
Copy link
Member

I think this is a doctest issue. Something to do with ‘+’ in the expected output?

@stevewanash
Copy link
Author

@iritkatriel Hey. Yes it's a doctest issue. I've gotten feedback on other platforms that unittest should work fine, and I bet it would, but there should be a set of rules with regard to doctest too, right? Should we emit the '+', should we emit the traceback stack as usual, should we include the entire thing? Cause none of these work currently. Also this issue only occurs with Exception groups, which is a fairly recent addition. Maybe it was an oversight.

@terryjreedy
Copy link
Member

Doctest ignores lines between Traceback (most recent call last) and one beginning with XyzError:. The convention is to not put in the ignored lines but to use on ... line instead, as in the top example in the doc.

    >>> factorial(-1)
    Traceback (most recent call last):
        ...
    ValueError: n must be >= 0

It needs a feature patch to recognize and initial exception group line and subsequent exception lines, with their prefixes.

>>> s([[2, 4], [6, 6, "f"]], 0)
  + Exception Group Traceback (most recent call last):
  |       ...
  | ExceptionGroup: There were errors (3 sub-exceptions)
  +-+---------------- 1 ----------------
    | TypeError: matrix must be a matrix (list of lists) of integers/floats
       ...
    | TypeError: Each row of the matrix must have the same size
    +---------------- 3 ----------------
    | ZeroDivisionError: division by zero

The above should be made to work, as is, cut and pasted, or with ignored lines replaced with '...' lines. The code for normal tracebacks should work with modification.

@terryjreedy
Copy link
Member

@stevewanash Would you like to try making a pull request?

@iritkatriel
Copy link
Member

iritkatriel commented Jun 5, 2023

Thank you, @terryjreedy for the pointers. I had a look and created #105294 to help with this issue. If nobody else wants to, I will work on this once the traceback feature is merged.

@sobolevn
Copy link
Member

I don't think that removing spacing, +, and - is safe here, at least in the initial implementation.
Since these two are different:

  + Exception Group Traceback (most recent call last):
  |   File "<stdin>", line 1, in <module>
  | ExceptionGroup: A (3 sub-exceptions)
  +-+---------------- 1 ----------------
    | TypeError: 1
    +---------------- 2 ----------------
    | ValueError: 2
    +---------------- 3 ----------------
    | ExceptionGroup: B (2 sub-exceptions)
    +-+---------------- 1 ----------------
      | IndexError: 3
      +---------------- 2 ----------------
      | IndexError: 4
      +------------------------------------

and

  + Exception Group Traceback (most recent call last):
  |   File "<stdin>", line 1, in <module>
  | ExceptionGroup: A (4 sub-exceptions)
  +-+---------------- 1 ----------------
    | TypeError: 1
    +---------------- 2 ----------------
    | ValueError: 2
    +---------------- 3 ----------------
    | ExceptionGroup: B (1 sub-exception)
    +-+---------------- 1 ----------------
      | IndexError: 3
      +------------------------------------
    +---------------- 4 ----------------
    | IndexError: 4
    +------------------------------------

So, I would be extra safe for now. Later we can add less verbose option.

Here's my plan:

  • Add new regex to parse exception groups, including nested ones
  • Add new Example method to compare parsed exception group to the raised one
  • Change format_exception_only to be def format_exception_only(exc, /, value=_sentinel, *, show_group=False):
  • Make sure ... is supported in the exception group traceback (instead of File "<stdin>", line 1, in <module>)

Right now I am working on this monster regex to parse parts of the exception group, wish me luck :)

@sobolevn
Copy link
Member

sobolevn commented Oct 25, 2023

Matching all cases with regex is almost impossible, there are so many corner cases:

  • Nested exception groups
  • Nested exceptions with tracebacks
  • Nested exceptions without a traceback
  • Notes on of all the things
  • Messages with new lines
  • Output limits and recursion errors
  • ... and full tracebacks which can be present
  • Potential SyntaxError instances with complex output

This is just the part of regular exceptions:

_EXCEPTION_GROUP_RE = re.compile(r"""
        # at first, we need to know the current identation
        ^(?P<indent1>\s+ )
        # at the same line it has the traceback information
        (?P<n> Traceback\ \( most\ recent\ call\ last \): \s*)
        # next, consume all traceback lines
        ^(?P<stack> \1 \s{2} .*?)
        # stop when the same indentation is found, followed by alpha-numeric
        ^(?P<msg> \1 \w+ .*?)
        # this is the line splitter
        ^\1\+-[\+-]--------------
""", re.VERBOSE | re.MULTILINE | re.DOTALL)

We also have special regexes for one-line exceptions, something like:

(?:\+-[\+-]--------------)(?:.*)
^(?P<single>\s+ \w+ .*)
(?:\+-[\+-]--------------)

and several others. Maintaining such regex will be quite complicated.

I think, that we need something else. Ideas?

Full-featured parser?
Smart splitting / .replace / etc?

@sobolevn
Copy link
Member

I think that I will experiment with a parser for this format.

@terryjreedy
Copy link
Member

terryjreedy commented Oct 26, 2023

Regexes cannot handle indefinite nesting.

I think it would be legitimate to just say that doctest cannot handle exception-group output, or that it can incompletely only test such. Tim Peters did not intend that it handle everything, and it has already been extended well beyond its original intention. In other words, I think that this is a feature addition rather than a bug fix.

This is not to say, Nikita, that you should give up now, but that it would be okay to decide at some point that there are better uses for your time.

@sobolevn
Copy link
Member

sobolevn commented Oct 26, 2023

I wrote a parser that does this:

text = """
  + Exception Group Traceback (most recent call last):
  |   File "<stdin>", line 1, in <module>
  | ExceptionGroup: A (5 sub-exceptions)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "<stdin>", line 3, in <module>
    |   File "<stdin>", line 2, in f
    | TypeError: 1
    | Happened in Iteration 1
    +---------------- 2 ----------------
    | ValueError: 2
    +---------------- 3 ----------------
    | ExceptionGroup: B (2 sub-exceptions)
    +-+---------------- 1 ----------------
      | IndexError: 3
      +---------------- 2 ----------------
      | IndexError: 4
      +------------------------------------
    +---------------- 4 ----------------
    | IndexError: 5
    +---------------- 5 ----------------
    |   File "x.py", line 23
    |     bad syntax
    | SyntaxError: error
    +------------------------------------
"""

x = ExceptionGroupParser(text[1:]).parse()

def f():
    exc = TypeError(1)
    exc.add_note('Happened in Iteration 1')
    raise exc

def raise_exc():
    try:
        f()
    except TypeError as e:
        exc = e
    return ExceptionGroup('A', [
        exc,
        ValueError(2),
        ExceptionGroup('B', [IndexError(3), IndexError(4)]),
        IndexError(5),
        SyntaxError("error", ("x.py", 23, None, "bad syntax")),
    ])

import traceback
y = traceback.format_exception_only(raise_exc(), show_group=True)

assert x == y  # passes 🎉 

So, I am surely not giving up just yet.

More tasks ahead:

  • Measuring performance of this parser - improving it, if needed
  • Testing corner-cases (there are lots of them)
  • Documenting everything
  • Improving error messages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants