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

gh-95914: Add paragraph about PEP 654 in main body of 'What's New in 3.11' #95937

Merged
merged 8 commits into from
Aug 13, 2022

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Aug 12, 2022

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir skip news awaiting core review labels Aug 12, 2022
@gvanrossum gvanrossum changed the title gh-95914: add paragraph about pep 654 in main body of 'what's new in 3.11' gh-95914: Add paragraph about pep 654 in main body of 'what's new in 3.11' Aug 12, 2022
@gvanrossum gvanrossum changed the title gh-95914: Add paragraph about pep 654 in main body of 'what's new in 3.11' gh-95914: Add paragraph about PEP 654 in main body of 'What's New in 3.11' Aug 12, 2022
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can add a link to except* in the docs, fine. Otherwise, fine too.

to raise and handle multiple unrelated exceptions simultaneously.
The builtin types :exc:`ExceptionGroup` and :exc:`BaseExceptionGroup`
make it possible to group exceptions and raise them together.
The new ``except*`` syntax generalizes ``except`` to match subgroups
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In line with our policy that PEPs are historic documents, not documentation, maybe this should deep link to the part under https://docs.python.org/3.11/reference/compound_stmts.html#try where except* is described?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both :keyword:`except* <except_star>` and :ref:`except* <except_star>` currently link to the beginning of the try section, where the except_star label is defined. For a deep link, the label should be moved down to the relevant paragraph.

Copy link
Member

@CAM-Gerlach CAM-Gerlach Aug 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the keyword appears to be linked precisely, both judging from its position in the source on both 3.12 and 3.11, and the fact that the index entry correctly links to it. But maybe a :keyword: ref doesn't?

However, the :ref: is indeed not; it would be good to move the non-try ones (that presumably don't rely on the section name for default link text, since it doesn't even mention except/else/finally) down to the appropriate passages.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a link to the top of the section is fine.


See :pep:`654` for more details.

(Contributed by Irit Katriel in :issue:`45292`. PEP written by
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(Contributed by Irit Katriel in :issue:`45292`. PEP written by
(Contributed by Irit Katriel in :gh:`89455`. PEP written by

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done (if at all) in a separate PR where the summary reference to the same issue is updated as well.

I say "if at all" because this is not historically accurate - I did the work under a bpo issue. Are you going to change all references to bpo issues in the docs to the gh issues they were mapped to?


See :pep:`678` for more details.

(Contributed by Irit Katriel in :issue:`45607`. PEP written by
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(Contributed by Irit Katriel in :issue:`45607`. PEP written by
(Contributed by Irit Katriel in :gh:`89770`. PEP written by

to raise and handle multiple unrelated exceptions simultaneously.
The builtin types :exc:`ExceptionGroup` and :exc:`BaseExceptionGroup`
make it possible to group exceptions and raise them together.
The new ``except*`` syntax generalizes ``except`` to match subgroups
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both :keyword:`except* <except_star>` and :ref:`except* <except_star>` currently link to the beginning of the try section, where the except_star label is defined. For a deep link, the label should be moved down to the relevant paragraph.

@CAM-Gerlach CAM-Gerlach added needs backport to 3.10 only security fixes 3.10 only security fixes needs backport to 3.11 only security fixes 3.11 only security fixes and removed needs backport to 3.10 only security fixes 3.10 only security fixes labels Aug 13, 2022
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @iritkatriel . A few relatively modest comments, mostly as applyable suggestions.

Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved
@@ -176,14 +176,33 @@ The :option:`-X` ``no_debug_ranges`` option and the environment variable
See :pep:`657` for more details. (Contributed by Pablo Galindo, Batuhan Taskaya
and Ammar Askar in :issue:`43950`.)

Exception Groups and ``except*`` (PEP 654)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we be consistent about PEP numbers in Feature section titles? Right now the type hint PEP sections are in the form PEP NNN: <Title>, this section and the one below is in the form <Title> (PEP NNN), and the two above it don't mention the PEP number at all in the title.

IMO, it would seem to make sense to either elide the PEP numbers in the New Features section titles (as I do for the section below in #95914 , since they don't add much value for most users and are already mentioned/linked in the section body and Summary), or consistently use them with the same format between these two sections (I can take care of the others in my separate PRs, but it would be nice to be consistent here).

to raise and handle multiple unrelated exceptions simultaneously.
The builtin types :exc:`ExceptionGroup` and :exc:`BaseExceptionGroup`
make it possible to group exceptions and raise them together.
The new ``except*`` syntax generalizes ``except`` to match subgroups
Copy link
Member

@CAM-Gerlach CAM-Gerlach Aug 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the keyword appears to be linked precisely, both judging from its position in the source on both 3.12 and 3.11, and the fact that the index entry correctly links to it. But maybe a :keyword: ref doesn't?

However, the :ref: is indeed not; it would be good to move the non-try ones (that presumably don't rely on the section name for default link text, since it doesn't even mention except/else/finally) down to the appropriate passages.

Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved
Comment on lines 200 to 205
default traceback.

See :pep:`678` for more details.

(Contributed by Irit Katriel in :issue:`45607`. PEP written by
Zac Hatfield-Dodds.)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make these changes in #95915 instead, since that PR is closer to the scope of the change (copyediting existing entries vs. adding a specific new one), and it will cause a significant merge conflict for whichever PR gets merged second otherwise?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I'll remove it and you can put it there.

iritkatriel and others added 7 commits August 13, 2022 12:18
Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
@iritkatriel iritkatriel merged commit 1402d2c into python:main Aug 13, 2022
@miss-islington
Copy link
Contributor

Thanks @iritkatriel for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@iritkatriel iritkatriel deleted the whatsnew branch August 13, 2022 11:49
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 13, 2022
…ew in 3.11' (pythonGH-95937)

(cherry picked from commit 1402d2c)

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@bedevere-bot
Copy link

GH-95954 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Aug 13, 2022
@CAM-Gerlach
Copy link
Member

A quick followup about the except* keyword—based on some testing by @ezio-melotti and myself, the index entry was a red herring; what actually happens with the :keyword: role is that it applies semantics and styling (i.e. literal, typically) but it just links to normal ref target labels, rather than ones created by a special directives, which the Sphinx docs also confirm. I'll open a separate PR to move the relevant ref target labels in compound_statement to more precise locations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants