Hi Cameron,

If you go long, I go longer :)


On Sun, Feb 28, 2021 at 10:51 PM Cameron Simpson <c...@cskk.id.au> wrote:

> On 28Feb2021 10:40, Irit Katriel <iritkatr...@googlemail.com> wrote:
> >split() and subgroup() take care to preserve the correct metadata on
> >all
> >the internal nodes, and if you just use them you only make safe
> operations.
> >This is why I am hesitating to add iteration utilities to the API. Like we
> >did, people will naturally try that first, and it's not the safest API.
>
> Wouldn't it be safe if the ExceptionGroup were immutable, as you plan?
> Or have you another hazard in mind?
>

Making them immutable won't help the metadata issue. split() and subgroup()
copy the (context, cause traceback) from the original ExceptionGroups (root
and internal nodes of the tree) to the result trees.   If you DIY creating
new
ExceptionGroups you need to take care of that.



> But all that said, being able to iterable the subexceptions seems a
> natural way to manage that:
>
>     unhandled = []
>     try:
>         .........
>     except *OSError as eg:
>         for e in eg:
>             if an ok exception:
>                 handle it
>             else:
>                 unhandled.append(e)
>     if unhandled:
>         raise ExceptionGroup("unhandled", unhandled)
>


You just lost the metadata of eg. It has no context, cause and its
traceback begins here. And the exceptions contained in it, if they came
from a deeper tree that you flattened into the list, now look like their
traceback jumps straight to here from the place they were actually first
inserted into an ExceptionGroup. This may well be an impossible code path.
Here's an example:

>>> import traceback
>>>
>>> def flatten(exc):
...     if isinstance(exc, ExceptionGroup):
...         for e in exc.errors:
...            yield from flatten(e)
...     else:
...         yield exc
...
>>> def f():
...     try:
...         raise ValueError(42)
...     except ValueError as e:
...         return e
...
>>> def g():
...     try:
...         raise ExceptionGroup("g", [f()])
...     except ExceptionGroup as e:
...         return e
...
>>> def h():
...     try:
...         raise ExceptionGroup("h", [g()])
...     except ExceptionGroup as e:
...         return e
...
>>> def flat_h():
...     try:
...         raise ExceptionGroup("flat_h", list(flatten(h())))
...     except ExceptionGroup as e:
...         return e
...
>>>
>>> traceback.print_exception(h())
Traceback (most recent call last):
  File "<stdin>", line 3, in h
ExceptionGroup: h
   ------------------------------------------------------------
   Traceback (most recent call last):
     File "<stdin>", line 3, in g
   ExceptionGroup: g
      ------------------------------------------------------------
      Traceback (most recent call last):
        File "<stdin>", line 3, in f
      ValueError: 42

>>> traceback.print_exception(flat_h())
Traceback (most recent call last):
  File "<stdin>", line 3, in flat_h
ExceptionGroup: flat_h
   ------------------------------------------------------------
   Traceback (most recent call last):
     File "<stdin>", line 3, in f
   ValueError: 42
>>>


traceback.print_exception(h()) prints a reasonable traceback - h() called
g() called f().

But according to  traceback.print_exception(flat_h()),   flat_h() called
f().


You can preserve the metadata (and the nested structure with all its
metadata) if you replace the last line with:
raise eg.subgroup(lambda e: e in  unhandled)
And for the part before that, iteration, Guido's pattern showed that you
can roll it into the subgroup callback.



>
> There are some immediate shortcomings above. In particular, I have no
> way of referencing the original ExceptionGroup without surprisingly
> cumbersome:


>     try:
>         .........
>     except ExceptionGroup as eg0:
>         unhandled = []
>         eg, os_eg = eg0.split(OSError)
>         if os_eg:
>             for e in os_eg:
>                 if an ok exception:
>                     handle it
>                 else:
>                     unhandled.append(e)
>         if eg:
>             eg, value_eg = eg.split(ValueError)
>             if value_eg:
>                 for e in value_eg:
>                     if some_ValueError_we_understand:
>                         handle it
>                     else:
>                         unhandled.append(e)
>         if eg:
>             unhandled.append(eg)
>         if unhandled:
>             raise ExceptionGroup("unhandled", unhandled) from eg0
>


This is where except* can help:

  try:
        .........
    except except *OSError as eg:
        unhandled = []
        handled, unhandled = eg.split(lambda e: e is an ok exception)  #
with side effect to handle e
        if unhandled:
            raise unhandled
    except *ValueError as eg:
        handled, unhandled = eg.split(lambda e: e is a value error we
understand)  # with side effect to handle e
        if  unhandled:


I have the following concerns with the pattern above:
>
> There's no way to make a _new_ ExceptionGroup with the same __cause__
> and __context__ and message as the original group: not that I can't
> assign to these, but I'd need to enuerate them; what if the exceptions
> grew new attributes I wanted to replicate?
>

As I said, split() and subgroup() do that for you.


> This cries out for another factory method like .subgroup but which makes
> a new ExceptionGroup from an original group, containing a new sequence
> of exceptions but the same message and coontext. Example:
>
>     unhandled_eg = eg0.with_exceptions(unhandled)
>

Why same message and context but not same cause and traceback?


>
> I don't see a documented way to access the group's message.
>

In the PEP: "The ExceptionGroup class exposes these parameters in the
fields message and errors".

(In my implementation it's still msg and not message, I need to change
that).


> I'm quite unhappy about .subgroup (and presumably .split) returning None
> when the group is empty. The requires me to have the gratuitous "if eg:"
> and "if value_eg:" if-statements in the example above.
>


> If, instead, ExceptionGroups were like any other container I could just
> test if they were empty:
>
>     if eg:
>
> _and_ even if they were empty, iterate over them. Who cares if the loop
> iterates zero times? The example code would become:
>
>     try:
>         .........
>     except ExceptionGroup as eg0:
>         unhandled = []
>         eg, os_eg = eg0.split(OSError)
>         for e in os_eg:
>             if an ok exception:
>                 handle it
>             else:
>                 unhandled.append(e)
>         eg, value_eg = eg.split(ValueError)
>         for e in value_eg:
>             if some_ValueError_we_understand:
>                 handle it
>             else:
>                 unhandled.append(e)
>         if eg:
>             unhandled.append(eg)
>         if unhandled:
>             raise eg0.with_exceptions(unhandled)
>
> You'll note that "except*" is not useful in this pattern.


I think it is useful, see above.


> However...
>
> If a subgroup had a reference to its parent this gets cleaner again:
>
>     unhandled = []
>     eg0 = None
>     try:
>         .........
>     except* OSError as os_eg:
>         eg0 = os_eg.__original__  # or __parent__ or something
>         for e in os_eg:
>             if an ok exception:
>                 handle it
>             else:
>     except* ValueError as value_eg:
>         eg0 = os_eg.__original__  # or __parent__ or something
>         for e in value_eg:
>             if some_ValueError_we_understand:
>                 handle it
>             else:
>                 unhandled.append(e)
>     except* Exception as eg:
>         eg0 = os_eg.__original__  # or __parent__ or something
>         unhandled.extend(eg)
>     if unhandled:
>         raise eg0.with_exceptions(unhandled)
>
> Except that here I have no way to get "eg0", the original
> ExceptionGroup, for the final raise without the additional .__original__
> attribute.
>
> Anyway, I'm strongly of the opinion that ExceptionGroups should look
> like containers, be iterable, be truthy/falsey based on empty/nonempty
> and that .split and .subgroup should return empty subgroups instead of
> None.
>


This would be true if iteration was a useful pattern for working with
ExceptionGroup, but I still think subgroup/split is a better tool in most
cases.



>
> Cheers,
> Cameron Simpson <c...@cskk.id.au>
>
_______________________________________________
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/7GHX66EXKHUU7OUAEJQFNW42VBLXKDPT/
Code of Conduct: http://python.org/psf/codeofconduct/

Reply via email to