Address feedback from Irit and Guido. - #5

Did we discuss whether the constructor takes a list of exceptions or uses varargs (*args)?

No, not yet, I believe. I guess we need to open a new issue to discuss the ExceptionGroup API in detail and later fold it into this proposal. @iritkatriel?

I think varargs is nice. In reality I think you would want to create it empty and add exceptions to it. But whichever way it's constructed - what happens when you try to raise an empty ExceptionGroup?

I think varargs is nice. In reality I think you would want to create it empty and add exceptions to it. But whichever way it's constructed - what happens when you try to raise an empty ExceptionGroup?

I've created an issue to discuss that: #6

Yikes! But why would it keep that group? If you start with ExceptionGroup(A(), B()) and you write

except *B as e:
    raise e

I hope you wouldn't end up with ExceptionGroup(A(), ExceptionGroup(B()))?

I hoped you would comment on this! I feel like this is one of the trickiest behaviors for us to define.

If you start with ExceptionGroup(A(), B()) and you write

except *B as e:
    raise e

I hope you wouldn't end up with ExceptionGroup(A(), ExceptionGroup(B()))?

I think it should end up with ExceptionGroup(A(), ExceptionGroup(B())). What if we change your code to:

except *B as e:
    e.marker = 'spam'
    raise e

I would expect the marker='spam' attribute to be present somewhere in what propagates out of the try block. And if we merge e back and raise ExceptionGroup(A(), B()) in the end this attribute will be lost.

What do you think?

Hm... But should you be doing that? In a sense ‘e’ is ephemeral, only for use in this one block. Surely if you mutate ‘e’ and then use bare raise it should get lost? (Hm, how can we describe bare raise? Ideally it should push the exceptions extracted back exactly from where they were extracted. Maybe the EG API needs to be described more carefully.)

Surely if you mutate ‘e’ and then use bare raise it should get lost?

That's a good point. So you propose that raise e would be equivalent to raise. If the user wants to "flatten" the hierarchy that would explicitly say raise ExceptionGroup(*e).

I like this idea because it makes it harder for users to accidentally flatten parts of their exceptions tree. The current wording was motivated by the e.marker = 'spam' example, but if we treat e as an ephemeral object we can perhaps ignore this use case.

This is also an area where a linter (or mypy, easily) can warn the user that they're working with an ephemeral object and that their changes or references to it will not persist the way they expect them to.

I've pushed another update erasing any semantic difference between raise and raise e.

Hm, you mean that bare raise doesn't update the traceback, whereas raise e would add that very "raise e" line to the traceback, right? Or is there something about e.__context__?

Both.

Guido, can you explain re __context__? I don't see any difference between raise and raise e here:

def foo():
    try:
        1 / 0
    except ZeroDivisionError:
        try:
            raise ValueError
        except Exception as e:
            raise e

try:
    foo()
except Exception as e:
    print(type(e), type(e.__context__))

# always prints "<class 'ValueError'> <class 'ZeroDivisionError'>"

or here:

def foo():
    try:
        1 / 0
    except ZeroDivisionError as e:
        raise e

try:
    foo()
except Exception as e:
    print(type(e), type(e.__context__))

# always prints "<class 'ZeroDivisionError'> <class 'NoneType'>"

It's not __context__ but the traceback that's different. Compare:

(v39) ~$ cat -n a.py; python a.py 
     1	def foo():
     2	    try:
     3	        1/0
     4	    except ZeroDivisionError as e:
     5	        raise e
     6	
     7	foo()
Traceback (most recent call last):
  File "/Users/guido/a.py", line 7, in <module>
    foo()
  File "/Users/guido/a.py", line 5, in foo
    raise e
  File "/Users/guido/a.py", line 3, in foo
    1/0
ZeroDivisionError: division by zero

to

(v39) ~$ cat -n b.py; python b.py 
     1	def foo():
     2	    try:
     3	        1/0
     4	    except ZeroDivisionError as e:
     5	        raise
     6	
     7	foo()
Traceback (most recent call last):
  File "/Users/guido/b.py", line 7, in <module>
    foo()
  File "/Users/guido/b.py", line 3, in foo
    1/0
ZeroDivisionError: division by zero

Got it, i'll push a commit clarifying this soon.

Unless you only allow return without expression that's not enough -- what would happen here?

try:
    raise ExceptionGroup(A(), B())
except *A:
    return 1
except *B:
    return 2

Right, great catch. I think we should only allow bare return. I'll add a clarification.

(FWIW I never use this feature myself)

what would happen here?

I think it would return 1, because A() would be matched first. But when running async code this is too fragile and would only allow users to write unpredictable code. IMO we should disable non-None returns.

There’s room to disagree here, but I would prefer not allowing any return statements over only allowing bare return. I worry that allowing bare return with the proposed semantics is closing the door for assigning any semantics to return with expression and for break/continue.

but I would prefer not allowing any return statements over only allowing bare return

TBH I'd prefer that too. As I said earlier I myself don't use this feature. Relying on a single "return" buried in finally or some except smells like bad code to me. But I'm also not sure how popular this idiom is: I can only recall seeing it once or twice in the wild.

I've just pushed an update prohibiting returns in except* altogether.

Done in 610b4db