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: Copyedit, link & format Typing Features section in 3.11 What's New #96097

Merged
merged 8 commits into from
Sep 19, 2022

Conversation

CAM-Gerlach
Copy link
Member

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

Part of #95913

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

Non-trivial highlights:

  • Split PEP 646 description into two paragraphs for easier readability
  • Clarify the syntax and semantics of the description of the interaction of total and required
  • Link the PEP 484 section implicitly referred by PEP 673 directly, as its quite non-trivial to find
  • Use a more Pythonically (and perhaps culturally) first/last name in PEP 681 example
  • Clarify PEP 563 status description, particularly that this is not the first deferral, the more common form as well as a link to the __future__ statement, and make the link text more descriptive

General changes:

  • Add Sphinx cross-references to additional items
  • Clarify the text in various spots
  • Fix/improve a number of other minor grammar, punctuation and phrasing issues
  • Consistent section heading capitalization, ref target labels and spacing

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir skip news awaiting review labels Aug 19, 2022
@CAM-Gerlach CAM-Gerlach marked this pull request as draft August 19, 2022 02:59
@CAM-Gerlach CAM-Gerlach force-pushed the whatsnew-copyedit-typing-features branch from 69dd4fd to dc75401 Compare August 19, 2022 03:01
@CAM-Gerlach CAM-Gerlach marked this pull request as ready for review August 19, 2022 03:02
@CAM-Gerlach CAM-Gerlach changed the title gh-95913: Copyedit, link & format New Typing Features section in 3.11 What's New gh-95913: Copyedit, link & format Typing Features section in 3.11 What's New Aug 19, 2022
@CAM-Gerlach CAM-Gerlach mentioned this pull request Aug 19, 2022
33 tasks
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks, mostly looks good!

Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved
m3: Movie = {"year": 2022} # error (missing required field title)
m1: Movie = {"title": "Black Panther", "year": 2018} # OK
m2: Movie = {"title": "Star Wars"} # OK (year is not required)
m3: Movie = {"year": 2022} # ERROR (missing required field title)
Copy link
Member

@AlexWaygood AlexWaygood Aug 19, 2022

Choose a reason for hiding this comment

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

What's the motivation for the ALLCAPS change here? It looks like shouting to me.

("OK" is fine but I'm not a fan of "ERROR".)

Copy link
Member Author

@CAM-Gerlach CAM-Gerlach Aug 19, 2022

Choose a reason for hiding this comment

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

It is intended to draw emphasis to the key contrast these examples are included to illustrate—the first and second are valid, while the third is invalid and produces an error. Also, "OK" is of course the correct, canonical capitalization of "OK", while it is "Error" is ALLCAPSed given the particular importance and best practice of prominently marking bad code as such.

Copy link
Member

Choose a reason for hiding this comment

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

"ERROR" still seems ugly and unnecessary to me; I don't really see the need for a change here.

Copy link
Member

@AlexWaygood AlexWaygood Aug 19, 2022

Choose a reason for hiding this comment

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

It also goes against the prevailing style in the typing docs — there are several # error comments, but none of them are ALLCAPS: https://docs.python.org/3/library/typing.html

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally, I value clarity and explicitness over subjective aesthetic preference, given a stated rationale. Also, I'm not sure how relevant the conventions of the typing stdlib doc are here, given it is a separate document from this one, and those particular choices may or may not have a clear, thought-out rationale.

But what do others think? @ezio-melotti @JelleZijlstra ?

If an all-caps ERROR is simply too objectionable, we can at least give it proper case

Suggested change
m3: Movie = {"year": 2022} # ERROR (missing required field title)
m3: Movie = {"year": 2022} # Error (missing required field title)

Copy link
Member Author

Choose a reason for hiding this comment

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

@AlexWaygood what about if we go with @ezio-melotti 's suggestion and not ALLCAPSing Error, like this?

Suggested change
m3: Movie = {"year": 2022} # ERROR (missing required field title)
m3: Movie = {"year": 2022} # Error (title is required, but missing)

Copy link
Member

Choose a reason for hiding this comment

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

@AlexWaygood what about if we go with @ezio-melotti 's suggestion and not ALLCAPSing Error, like this?

Sounds good to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

For inline comments like these I tend to use all lowercase and no full stops at the end, so even ok and error would be fine with me.

Same, I don't add a period at the end either for inline or single-line comments...but I notice you do use period for titles (and commit summary lines), which slightly confuses me :) (To note, I do start them with a capital letter, which follows the examples in PEP 8 and avoids looking (IMO) sloppy—though that latter point is personal aesthetic preference)

Copy link
Member

Choose a reason for hiding this comment

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

FWIW, in this general document, I'd appreciate a subtle indication that this is a typing error -- not a SyntaxError or runtime exception. Perhaps Wrong, in whatever case you prefer.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a great point—to be more specific, we could say something like Type check error or Incompatible type

Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.11.rst Outdated Show resolved Hide resolved
Co-authored-by: Ezio Melotti <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks! I still don't really think there's a good rationale for changing # error to # ERROR, but that's a minor nit :)

@encukou
Copy link
Member

encukou commented Sep 19, 2022

Merging, the minor nit can be resolved later.

@encukou encukou merged commit 558768f 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.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 19, 2022
…11 What's New (pythonGH-96097)

Co-authored-by: Ezio Melotti <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
(cherry picked from commit 558768f)

Co-authored-by: C.A.M. Gerlach <[email protected]>
@bedevere-bot
Copy link

GH-96934 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 Sep 19, 2022
miss-islington added a commit that referenced this pull request Sep 19, 2022
…t's New (GH-96097)

Co-authored-by: Ezio Melotti <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
(cherry picked from commit 558768f)

Co-authored-by: C.A.M. Gerlach <[email protected]>
pablogsal pushed a commit that referenced this pull request Oct 22, 2022
…t's New (GH-96097)

Co-authored-by: Ezio Melotti <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
(cherry picked from commit 558768f)

Co-authored-by: C.A.M. Gerlach <[email protected]>
pablogsal pushed a commit that referenced this pull request Oct 22, 2022
…t's New (GH-96097)

Co-authored-by: Ezio Melotti <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
(cherry picked from commit 558768f)

Co-authored-by: C.A.M. Gerlach <[email protected]>
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