GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — PEP 670: clarify cast; don't change return type by vstinner · Pull Request #2349 · python/peps

Conversation

@vstinner

@vstinner vstinner commented Feb 21, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Clarify how arguments are cast
  • Limited C API 3.11 no longer cast arguments
  • No longer remove return value
  • Require to not change the return type
  • Don't change macros having multiple return types

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 21, 2022
@vstinner
PEP 670: clarify cast; don't change return type
9a22e13 
* Clarify how arguments are cast
* Limited C API 3.11 no longer cast arguments
* No longer remove return value
* Require to not change the return type
* Don't change macros having multiple return types
@vstinner

vstinner commented Feb 21, 2022

Copy link
Copy Markdown
Member Author

I updated PEP 670 to take in account @encukou and @gpshead feedback on python-dev and on my python/cpython#31221 review.

@encukou @gpshead @erlend-aasland: Would you mind to have a look?
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Feb 21, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

You can read the formatted PEP by clicking in the Files Changes tab on the 3 middle dots (...) menu at the pep-0670.rst line: in this menu, click on "View file".

Current link: pep-0670.rst.
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner added 2 commits February 21, 2022 18:19
@vstinner
Rephrase
977cde6
@vstinner
Typ
a1eae58
@CAM-Gerlach
CAM-Gerlach requested review from encukou, erlend-aasland and gpshead February 21, 2022 19:06
CAM-Gerlach
CAM-Gerlach requested changes Feb 21, 2022
View reviewed changes

@CAM-Gerlach CAM-Gerlach left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixes for a number of grammar errors in the added/changed text, as well as a few PEP 12 issues and for correct code block syntax highlighting. All the changes are as suggestions, and can be applied cleanly together by simply going to the Files tab, clicking the Add to batch button on each change, and then clicking the Commit button, which will avoid the all problems we both experienced last time. Alternatively, I'm happy to do it for you. Either way, don't forget to pull your branch after doing so. Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread pep-0670.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread pep-0670.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread pep-0670.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread pep-0670.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread pep-0670.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread pep-0670.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread pep-0670.rst
* ``_Py_atomic_store_64bit()``
* ``asdl_seq_SET()``
* ``asdl_seq_SET_UNTYPED()``
For example, Python 3.7 changed ``PyUnicode_AsUTF8()`` return type from

@CAM-Gerlach CAM-Gerlach Feb 21, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
For example, Python 3.7 changed ``PyUnicode_AsUTF8()`` return type from
For example, Python 3.7 changed ``PyUnicode_AsUTF8()``\ 's return type from

Fix grammar error

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@CAM-Gerlach CAM-Gerlach Feb 22, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This grammar error wasn't fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread pep-0670.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread pep-0670.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread pep-0670.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland
erlend-aasland approved these changes Feb 22, 2022
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm fine with this, Victor.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
vstinner added 3 commits February 22, 2022 13:08
@vstinner
Address CAM's review
dde12d3
@vstinner
Address CAM's review
ba4aadf
@vstinner
Fix Sphinx
08d57fa
@vstinner

vstinner commented Feb 22, 2022

Copy link
Copy Markdown
Member Author

@CAM-Gerlach: I addressed most of your reviews, thanks.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner merged commit 42e921e into python:main Feb 22, 2022
@vstinner
vstinner deleted the pep670_cast branch February 22, 2022 12:17
@vstinner

vstinner commented Feb 22, 2022

Copy link
Copy Markdown
Member Author

Follow-up: I wrote an email to python-dev: https://mail.python.org/archives/list/[email protected]/thread/VM6I3UHVMME6QRSUOYLK6N2OZHP454W6/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@CAM-Gerlach

CAM-Gerlach commented Feb 22, 2022 •
edited by AA-Turner
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Good call on using the language:: c directive in lieu of .. code-block:: c on all the code blocks, but it looks like at least a few of the grammar errors cited were not fixed (FYI, you can edit them, e.g. to remove the code-block directive, and apply those instead, to avoid them getting missed in the future). If I do get the chance to do a full copyedit of the PEP (I've been busy with other PEP editor stuff, sorry), I'll clean those up.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland erlend-aasland mentioned this pull request May 13, 2022
pep8/greppable exception messages erlend-aasland/peps#1
Closed
@erlend-aasland erlend-aasland mentioned this pull request Jun 27, 2022
pep 687/mark as accepted erlend-aasland/peps#2
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CAM-Gerlach CAM-Gerlach CAM-Gerlach requested changes
@erlend-aasland erlend-aasland erlend-aasland approved these changes
@encukou encukou Awaiting requested review from encukou
@gpshead gpshead Awaiting requested review from gpshead

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

CLA signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@vstinner @CAM-Gerlach @erlend-aasland @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.