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-95913: Fix and copyedit New Features section of 3.11 What's New #95915

Merged
merged 5 commits into from
Sep 19, 2022

Conversation

CAM-Gerlach
Copy link
Member

@CAM-Gerlach CAM-Gerlach commented Aug 12, 2022

Part of #95913

Fixes Sphinx and textual issues in the What's New in Python 3.11 "New Features" section, improves the clarity, quality and non-duplication of the text, and links to referenced documentation sections.

Nothing too major, but a few highlights:

  • Consolidate the traceback location API section with the rest of the exception location section, to avoid significant duplication of nearly half the section's contents between them
  • Fix the heading level of the "Exception notes" feature description; it was mistakenly under the "Exact Error Locations" section
  • Fix a broken reference to the critical BaseException.add_note() method
  • Sphinx-link to referenced items, including code objects, bytecode definition and bytecode instructions
  • Fix/improve a number of other minor grammar, punctuation and phrasing issues

Comment on lines +106 to 130
when dealing with deeply nested :class:`dict` objects and multiple function calls:

.. code-block:: python
Copy link
Member

Choose a reason for hiding this comment

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

Can't we use this here?

Suggested change
when dealing with deeply nested :class:`dict` objects and multiple function calls:
.. code-block:: python
when dealing with deeply nested :class:`dict` objects and multiple function calls::

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 didn't see a strong reason to change this from the original source, since it renders equivalently, is a little more explicit and (IIRC) warns if the syntax used doesn't actually lex according to the declared lexer (vs. :: fails silently). But I can if you think its important

.. index:: object; code, code object

.. _codeobjects:

Copy link
Member

Choose a reason for hiding this comment

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

How does this change affect :ref: and index entries linking (if it does)?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't affect the index entries (it still links to the same place, right above the section) but fixes an issue with the ref target not actually being the section as intended, and thus not having a generated link text causing :ref:`codeobjects` to error out. It won't affect any existing refs, since it only changes the generated link text and not the ref target location, which the existing refs could not be using or they would have errored and failed the build.

Comment on lines +124 to 148
As well as complex arithmetic expressions:

.. code-block:: python
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
As well as complex arithmetic expressions:
.. code-block:: python
As well as complex arithmetic expressions::

Same here, and possibly elsewhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

See above—I can do this if you really think its necessary, but I'm not sure it is.

Doc/whatsnew/3.11.rst Show resolved Hide resolved
that is not available at the time when the exception is raised.
The added notes appear in the default traceback.
See :pep:`678` for more details.
(Contributed by Irit Katriel in :issue:`45607`.)
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`.)
(Contributed by Irit Katriel in :gh:`89770`.)

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 thought about this, but I didn't end up updating these, since the BPO issue was canonical at the time the work was done, and the links redirect to the up to date GH issue. Is there any guidance on whether old :issue: refs should be manually converted, or left as-is? I couldn't find any in the devguide; if not, it might be better to make this a separate issue/discussion and be done consistently throughout.

Copy link
Member

Choose a reason for hiding this comment

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

IMO it's best to avoid links to redirects, but it's not a big deal.

Copy link
Member Author

@CAM-Gerlach CAM-Gerlach Sep 19, 2022

Choose a reason for hiding this comment

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

Yup, if people agree I'm happy to go through and convert them; I just think its best to do it in a separate PR after these more important ones are all merged, so it can be decided there, its consistent and we don't hold up the rest of the changes since we're on the clock here.


The :option:`-X` ``no_debug_ranges`` option and the environment variable
:envvar:`PYTHONNODEBUGRANGES` can be used to disable this feature.
- The :c:func:`PyCode_Addr2Location` function in the C API.

See :pep:`657` for more details. (Contributed by Pablo Galindo, Batuhan Taskaya
and Ammar Askar in :issue:`43950`.)
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
and Ammar Askar in :issue:`43950`.)
and Ammar Askar in :gh:`88116`.)

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

@iritkatriel
Copy link
Member

Approved the pep 678 related edits.

@CAM-Gerlach
Copy link
Member Author

Thanks @iritkatriel and sorry for the delay getting to this @ezio-melotti — I responded to the comments, fixed the conflict caused by #95937 changing the title of the next section, cherry-picked in @iritkatriel 's #95955 (to fix the conflicts with this), and added a commit adding consistent, standardized section ref labels to the touched sections.

Closes #95955

@Fidget-Spinner
Copy link
Member

LGTM other than the discussion about bpo vs GH issue linking being unresolved.

@encukou
Copy link
Member

encukou commented Sep 19, 2022

This is an improvement, so I'll merge it.
Issue linking can be fixed later.

@encukou encukou merged commit 8ee27e3 into python:main Sep 19, 2022
@miss-islington
Copy link
Contributor

Thanks @CAM-Gerlach for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@bedevere-bot
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 19, 2022
miss-islington added a commit that referenced this pull request Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants