gh-120521: clarify except* documentation to allow tuples #120523

yangdanny97 · 2024-06-14T20:48:09Z

fixes #120521

Discussion link: https://discuss.python.org/t/clarifying-typing-behavior-for-exception-handlers-except-except/55060/2

Issue: Clarify documentation for except* #120521

📚 Documentation preview 📚: https://cpython-previews--120523.org.readthedocs.build/

ghost · 2024-06-14T20:48:12Z

All commit authors signed the Contributor License Agreement.

JelleZijlstra · 2024-06-14T22:07:58Z

Doc/reference/compound_stmts.rst

@@ -378,8 +378,8 @@ exception group with an empty message string. ::
   ...
   ExceptionGroup('', (BlockingIOError()))

-An :keyword:`!except*` clause must have a matching type,
-and this type cannot be a subclass of :exc:`BaseExceptionGroup`.
+An :keyword:`!except*` clause must have a matching type or a tuple of matching


It might be worth expanding this further:

The docs for except (the previous section) never say that each type must be a subtype of BaseException. That's probably worth adding.

One important difference between except and except* is that the former allows "bare" except clauses (just except:), but except* doesn't. I think that's worth mentioning in a sentence by itself.

We don't say what happens if the type is a subclass of BaseExceptionGroup.

So I'd suggest something like this:

Suggested change

An :keyword:`!except*` clause must have a matching type or a tuple of matching

Unlike :keyword:`except` clauses, :keyword:`!except*` clauses must have a matching

expression; `except*:` is not valid syntax. When the handler is evaluated, the

expression must evaluate to a type or a tuple containing types. Each type must be a

subclass of :exc:`BaseException` and must not be a subclass of :exc:`BaseExceptionGroup`.

Okay, I've added a mention that the runtime will raise a TypeError if the type is wrong, and updated the except section to have similar language.

Should the latter be split into a separate PR, or should I just update the title/summary of this PR?

JelleZijlstra

This looks right, cc @iritkatriel for except*.

Doc/reference/compound_stmts.rst

Co-authored-by: Jelle Zijlstra <[email protected]>

Doc/reference/compound_stmts.rst

Co-authored-by: Irit Katriel <[email protected]>

offensive-vk

Nice explanation!

iritkatriel · 2024-06-15T14:05:24Z

I think this PR should just resolve the issue it is associated with. As it is now it is doing a lot more than that.
For example, the fact that an incorrect exception expression results in TypeError at runtime is an implementation detail, it is not currently (as far as I know) documented in the language spec.

Can you limit the PR to just resolve the problem raised in the issue?

yangdanny97 · 2024-06-15T19:16:30Z

I think this PR should just resolve the issue it is associated with. As it is now it is doing a lot more than that. For example, the fact that an incorrect exception expression results in TypeError at runtime is an implementation detail, it is not currently (as far as I know) documented in the language spec.

Can you limit the PR to just resolve the problem raised in the issue?

I'll remove the TypeError part, though I think the rest of it is kind of difficult to untangle because the except* docs now references the except docs (with the "in addition to"/"unlike" wording), so it might make sense to keep the changes together.

Do you have a preference for how to split it up?

Doc/reference/compound_stmts.rst

iritkatriel

I think it became a bit wordy. I tried to reword it more concisely.

Doc/reference/compound_stmts.rst

iritkatriel · 2024-06-17T14:40:47Z

Doc/reference/compound_stmts.rst

+matching expression; ``except*:`` is not valid syntax. In addition to the
+requirements for matching expressions in :keyword:`except` clauses, the
+types in the matching expression of an :keyword:`!except*` clause cannot
+be :exc:`BaseExceptionGroup` or a subclass of :exc:`BaseExceptionGroup`.


(Above we talk about "exception group" when we refer to BaseException and its subclasses).

Sorry, I'm confused about this comment - is it addressed in your suggestions already?

The other suggestions on this PR which I've merged in seem to have removed all references to BaseException and BaseExceptionGroup classnames in this doc.

Feels like explicitly mentioning the class names would be more clear, wouldn't it?

Yes, this comment is addressed.

Since we talk about "exception group" in the previous paragraphs, assuming the reader knows what we mean there, I don't think specifying the type here would add clarity.

Co-authored-by: Irit Katriel <[email protected]>

iritkatriel

LGTM, I'll let @JelleZijlstra have another look.

JelleZijlstra

Looks good, just a wording suggestion

Doc/reference/compound_stmts.rst

JelleZijlstra · 2024-06-19T12:46:37Z

Doc/reference/compound_stmts.rst

+For an :keyword:`!except` clause with an expression, the
+expression must evaluate to an exception type or a tuple of exception types.
+The raised exception matches an :keyword:`!except` clause whose expression evaluates
+to the class or a :term:`non-virtual base class <abstract base class>` of the exception object,


This is a pre-existing problem and doesn't have to be solved in this PR, but I don't think linking to the definition of "abstract base class" here is great.

Co-authored-by: Jelle Zijlstra <[email protected]>

miss-islington-app · 2024-06-19T18:49:04Z

Thanks @yangdanny97 for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

…nGH-120523) (cherry picked from commit 58b3f11) Co-authored-by: Danny Yang <[email protected]> Co-authored-by: Jelle Zijlstra <[email protected]> Co-authored-by: Irit Katriel <[email protected]>

bedevere-app · 2024-06-19T18:49:15Z

GH-120750 is a backport of this pull request to the 3.13 branch.

…nGH-120523) (cherry picked from commit 58b3f11) Co-authored-by: Danny Yang <[email protected]> Co-authored-by: Jelle Zijlstra <[email protected]> Co-authored-by: Irit Katriel <[email protected]>

bedevere-app · 2024-06-19T18:49:20Z

GH-120751 is a backport of this pull request to the 3.12 branch.

…20523) (#120751) (cherry picked from commit 58b3f11) Co-authored-by: Danny Yang <[email protected]> Co-authored-by: Jelle Zijlstra <[email protected]> Co-authored-by: Irit Katriel <[email protected]>

…20523) (#120750) (cherry picked from commit 58b3f11) Co-authored-by: Danny Yang <[email protected]> Co-authored-by: Jelle Zijlstra <[email protected]> Co-authored-by: Irit Katriel <[email protected]>

…n#120523) Co-authored-by: Jelle Zijlstra <[email protected]> Co-authored-by: Irit Katriel <[email protected]>

bedevere-app bot mentioned this pull request Jun 14, 2024

Clarify documentation for except* #120521

Closed

bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Jun 14, 2024

JelleZijlstra self-requested a review June 14, 2024 21:01

JelleZijlstra reviewed Jun 14, 2024

View reviewed changes

clarify except* documentation

7979097

yangdanny97 force-pushed the main branch from 4c8e8ea to 7979097 Compare June 15, 2024 00:23

fix snippet

4a58d67

JelleZijlstra reviewed Jun 15, 2024

View reviewed changes

Doc/reference/compound_stmts.rst Outdated Show resolved Hide resolved

Doc/reference/compound_stmts.rst Outdated Show resolved Hide resolved

yangdanny97 and others added 2 commits June 15, 2024 06:55

Update Doc/reference/compound_stmts.rst

65e5286

Co-authored-by: Jelle Zijlstra <[email protected]>

Update Doc/reference/compound_stmts.rst

fd3a79e

Co-authored-by: Jelle Zijlstra <[email protected]>

iritkatriel reviewed Jun 15, 2024

View reviewed changes

Doc/reference/compound_stmts.rst Outdated Show resolved Hide resolved

Doc/reference/compound_stmts.rst Outdated Show resolved Hide resolved

Doc/reference/compound_stmts.rst Outdated Show resolved Hide resolved

Doc/reference/compound_stmts.rst Outdated Show resolved Hide resolved

yangdanny97 and others added 2 commits June 15, 2024 09:04

Update Doc/reference/compound_stmts.rst

863b59e

Co-authored-by: Irit Katriel <[email protected]>

clarify wording and reduce duplication

33733a8

offensive-vk approved these changes Jun 15, 2024

View reviewed changes

bedevere-app bot added awaiting core review and removed awaiting review labels Jun 15, 2024

remove reference to runtime TypeError

cd988b9

iritkatriel reviewed Jun 17, 2024

View reviewed changes

Doc/reference/compound_stmts.rst Outdated Show resolved Hide resolved

iritkatriel reviewed Jun 17, 2024

View reviewed changes

yangdanny97 and others added 3 commits June 17, 2024 10:58

Update Doc/reference/compound_stmts.rst

5d34d69

Co-authored-by: Irit Katriel <[email protected]>

Update Doc/reference/compound_stmts.rst

2ccbb8a

Co-authored-by: Irit Katriel <[email protected]>

Update Doc/reference/compound_stmts.rst

2540bca

Co-authored-by: Irit Katriel <[email protected]>

iritkatriel approved these changes Jun 19, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Jun 19, 2024

JelleZijlstra approved these changes Jun 19, 2024

View reviewed changes

Update Doc/reference/compound_stmts.rst

e74c879

Co-authored-by: Jelle Zijlstra <[email protected]>

JelleZijlstra approved these changes Jun 19, 2024

View reviewed changes

JelleZijlstra enabled auto-merge (squash) June 19, 2024 18:43

JelleZijlstra added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Jun 19, 2024

JelleZijlstra merged commit 58b3f11 into python:main Jun 19, 2024
27 checks passed

bedevere-app bot removed the awaiting merge label Jun 19, 2024

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jun 19, 2024

bedevere-app bot removed the needs backport to 3.12 only security fixes label Jun 19, 2024

mrahtz pushed a commit to mrahtz/cpython that referenced this pull request Jun 30, 2024

pythongh-120521: clarify except* documentation to allow tuples (pytho…

a104fc6

…n#120523) Co-authored-by: Jelle Zijlstra <[email protected]> Co-authored-by: Irit Katriel <[email protected]>

noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024

pythongh-120521: clarify except* documentation to allow tuples (pytho…

1023304

…n#120523) Co-authored-by: Jelle Zijlstra <[email protected]> Co-authored-by: Irit Katriel <[email protected]>

estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024

pythongh-120521: clarify except* documentation to allow tuples (pytho…

1662e86

…n#120523) Co-authored-by: Jelle Zijlstra <[email protected]> Co-authored-by: Irit Katriel <[email protected]>

-An :keyword:`!except*` clause must have a matching type or a tuple of matching
+Unlike :keyword:`except` clauses, :keyword:`!except*` clauses must have a matching
+expression; `except*:` is not valid syntax. When the handler is evaluated, the
+expression must evaluate to a type or a tuple containing types. Each type must be a
+subclass of :exc:`BaseException` and must not be a subclass of :exc:`BaseExceptionGroup`.

Uh oh!

gh-120521: clarify except* documentation to allow tuples #120523

gh-120521: clarify except* documentation to allow tuples #120523

Uh oh!

Conversation

yangdanny97 commented Jun 14, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jun 14, 2024 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JelleZijlstra Jun 14, 2024

Choose a reason for hiding this comment

Uh oh!

yangdanny97 Jun 15, 2024

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

offensive-vk left a comment

Choose a reason for hiding this comment

Uh oh!

iritkatriel commented Jun 15, 2024

Uh oh!

yangdanny97 commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

iritkatriel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iritkatriel Jun 17, 2024

Choose a reason for hiding this comment

Uh oh!

yangdanny97 Jun 19, 2024

Choose a reason for hiding this comment

Uh oh!

iritkatriel Jun 19, 2024

Choose a reason for hiding this comment

Uh oh!

iritkatriel left a comment

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JelleZijlstra Jun 19, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington-app bot commented Jun 19, 2024

Uh oh!

bedevere-app bot commented Jun 19, 2024

Uh oh!

bedevere-app bot commented Jun 19, 2024

Uh oh!

Uh oh!

yangdanny97 commented Jun 14, 2024 •

edited by github-actions bot

Loading

ghost commented Jun 14, 2024 •

edited by ghost

Loading

yangdanny97 commented Jun 15, 2024 •

edited

Loading