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 — bpo-40943: PY_SSIZE_T_CLEAN required for '#' formats by vstinner · Pull Request #20784 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Jun 10, 2020 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

The PY_SSIZE_T_CLEAN macro must now be defined to use
PyArg_ParseTuple() and Py_BuildValue() "#" formats: "es#", "et#",
"s#", "u#", "y#", "z#", "U#" and "Z#". See the PEP 353.

https://bugs.python.org/issue40943

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
vstinner requested review from methane and serhiy-storchaka June 10, 2020 16:38
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jun 10, 2020
@bedevere-bot bedevere-bot added the awaiting core review label Jun 10, 2020
@vstinner

vstinner commented Jun 10, 2020

Copy link
Copy Markdown
Member Author

@methane @serhiy-storchaka: The feature is deprecated in Python 3.8 and Python 3.9. Are you ok to drop it now? Would you mind to review my PR?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented Jun 10, 2020

Copy link
Copy Markdown
Member

To compensate the shorter release cycle we should use longer deprecation period. Also, breaking changes in the C API have higher bar than breaking changes in Python code. Although I don't think that it will cause severe troubles in this case, because it is easy to write code which works with all supported Python versions, up to very old ones. But 2 years is just too little.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka reviewed Jun 10, 2020
View reviewed changes
Comment thread Python/getargs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.10.rst Resolved

Uh oh!

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

vstinner commented Jun 10, 2020

Copy link
Copy Markdown
Member Author

To compensate the shorter release cycle we should use longer deprecation period. Also, breaking changes in the C API have higher bar than breaking changes in Python code. Although I don't think that it will cause severe troubles in this case, because it is easy to write code which works with all supported Python versions, up to very old ones. But 2 years is just too little.

@benjaminp and @brettcannon plan to update https://www.python.org/dev/peps/pep-0387/ to require 2 releases instead of 1 between the addition of DeprecationWarning and the removal of the feature, since Python release cycle is now 12 months instead of 18 months (https://www.python.org/dev/peps/pep-0602/).

But here, the warning is already in 2 releases: Python 3.8 and Python 3.9, no?

Note: I would prefer to make such incompatible changes at the beginning of the development cycle, to have more chances to discover issues in third party projects. So we will have more time to decide if the incompatible change must wait longer and so be reverted.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
bpo-40943: PY_SSIZE_T_CLEAN required for '#' formats
1267c84 
The PY_SSIZE_T_CLEAN macro must now be defined to use
PyArg_ParseTuple() and Py_BuildValue() "#" formats: "es#", "et#",
"s#", "u#", "y#", "z#", "U#" and "Z#". See the PEP 353.

Update _testcapi.test_buildvalue_issue38913().
@vstinner

vstinner commented Jun 10, 2020

Copy link
Copy Markdown
Member Author

I checked Cython and lxml.

Cython always generate #define PY_SSIZE_T_CLEAN. So all C extensions built by Cython are not affected by the change.

lxml test suite pass with this change (ignoring getiterator errors, unrelated to this PR).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
methane
methane approved these changes Jun 12, 2020
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jun 12, 2020
@vstinner

vstinner commented Jun 15, 2020

Copy link
Copy Markdown
Member Author

@serhiy-storchaka: I completed my PR to also change Py_BuildValue(). Would you mind to review the updated PR?
All reactions

Sorry, something went wrong.

Uh oh!

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

vstinner commented Jun 17, 2020

Copy link
Copy Markdown
Member Author

If @serhiy-storchaka doesn't reply, I plan to merge this PR at the end of the week (friday).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
vstinner merged commit 37bb289 into python:master Jun 19, 2020
@bedevere-bot bedevere-bot removed the awaiting merge label Jun 19, 2020
@vstinner
vstinner deleted the ssize_t_clean branch June 19, 2020 09:45
@vstinner

vstinner commented Jun 19, 2020

Copy link
Copy Markdown
Member Author

Thanks for the review @methane! I prefer to merge such incompatible change at the beginning of 3.10 dev cycle.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
fasih pushed a commit to fasih/cpython that referenced this pull request Jun 29, 2020
@vstinner @fasihahmad
bpo-40943: PY_SSIZE_T_CLEAN required for '#' formats (pythonGH-20784)
b1e6151 
The PY_SSIZE_T_CLEAN macro must now be defined to use
PyArg_ParseTuple() and Py_BuildValue() "#" formats: "es#", "et#",
"s#", "u#", "y#", "z#", "U#" and "Z#". See the PEP 353.

Update _testcapi.test_buildvalue_issue38913().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@methane methane methane approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

5 participants

@vstinner @serhiy-storchaka @methane @the-knights-who-say-ni @bedevere-bot
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.