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 — gh-108494: Argument Clinic: Document how to generate code that uses the limited C API by vstinner · Pull Request #108584 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Aug 28, 2023 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Remove also the --limited command line option. Now the limited C API is only used if the source code contains "#define Py_LIMITED_API".

  • Issue: Add support for the limited C API to Argument Clinic #108494

📚 Documentation preview 📚: https://cpython-previews--108584.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner vstinner added the skip news label Aug 28, 2023
@vstinner
vstinner requested review from AlexWaygood and erlend-aasland as code owners August 28, 2023 20:41
@bedevere-bot bedevere-bot mentioned this pull request Aug 28, 2023
Add support for the limited C API to Argument Clinic #108494
Closed
@bedevere-bot bedevere-bot added the awaiting core review label Aug 28, 2023
@vstinner vstinner mentioned this pull request Aug 28, 2023
gh-108494: AC supports pos-only args in limited C API #108498
Merged
@vstinner

vstinner commented Aug 28, 2023

Copy link
Copy Markdown
Member Author

Remove also the --limited command line option. Now the limited C API is only used if the source code contains "#define Py_LIMITED_API".

I converted locally multiple C extensions to the limited C API. Since the limited C API is now automatically enabled by "#define Py_LIMITED_API", in fact, the --limited command line option is no longer needed. It "just works" :-)
erlend-aasland reacted with thumbs up emoji AlexWaygood reacted with hooray emoji
All reactions
  • 👍 1 reaction
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland erlend-aasland changed the title gh-108494: AC: Document How to use the Limited C API gh-108494: Argument Clinic: Automatically determine if the Limited API is used Aug 29, 2023
erlend-aasland
erlend-aasland reviewed Aug 29, 2023
View reviewed changes
Comment thread Doc/howto/clinic.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood
AlexWaygood reviewed Aug 29, 2023
View reviewed changes
Comment thread Doc/howto/clinic.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka serhiy-storchaka added the topic-argument-clinic label Aug 29, 2023
@serhiy-storchaka

serhiy-storchaka commented Aug 29, 2023

Copy link
Copy Markdown
Member

Please do not remove the --limited command line option. It is very conventional for testing. I couldn't do so much without it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Please do not remove the --limited command line option. It is very conventional for testing. I couldn't do so much without it.

The --limited command-line option is convenient for testing purposed, but the approach proposed here is convenient for the build system 🤔
All reactions

Sorry, something went wrong.

Uh oh!

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

AlexWaygood commented Aug 29, 2023

Copy link
Copy Markdown
Member

I don't have a strong opinion either way. I don't see any harm in keeping it, if Serhiy prefers to
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.
@erlend-aasland

erlend-aasland commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

I don't have a strong opinion either way. I don't see any harm in keeping it, if Serhiy prefers to

Yeah, of course you can have both; Argument Clinic can try to autodetect it, and you can override it using the CLI.
AlexWaygood 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 August 29, 2023 17:15
@vstinner
pythongh-108494: AC: Document How to use the Limited C API
a8ba2b8
@vstinner
Address reviews
96502df
@vstinner
vstinner force-pushed the clinic_limited_doc branch from 5222084 to 96502df Compare August 29, 2023 15:19
@vstinner

vstinner commented Aug 29, 2023

Copy link
Copy Markdown
Member Author

I updated the PR:

  • Keep --limited option and document it
  • Address @erlend-aasland's review and @AlexWaygood's review
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood
AlexWaygood reviewed Aug 29, 2023
View reviewed changes
Comment thread Doc/howto/clinic.rst Resolved

Uh oh!

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

vstinner commented Aug 29, 2023

Copy link
Copy Markdown
Member Author

Please do not remove the --limited command line option. It is very conventional for testing. I couldn't do so much without it.

If you need it, sure, I can keep it. It's just that for my needs, I didn't use it anymore. I was easier to remove it rather document it :-D
All reactions

Sorry, something went wrong.

Uh oh!

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

vstinner commented Aug 29, 2023

Copy link
Copy Markdown
Member Author

@erlend-aasland erlend-aasland changed the title gh-108494: AC: Document How to use the Limited C API gh-108494: Argument Clinic: Automatically determine if the Limited API is used Aug 29, 2023

I'm not sure that this PR title. Usage of the Limited C API is already determined automatically in the main branch, I did it in a previous change. This PR is more about documenting my recent AC work.
All reactions

Sorry, something went wrong.

Uh oh!

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

serhiy-storchaka commented Aug 29, 2023

Copy link
Copy Markdown
Member

You could left it undocumented if only I use it. 😄
erlend-aasland reacted with laugh emoji
All reactions
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner @AlexWaygood
Update Doc/howto/clinic.rst
8b2c572 
Co-authored-by: Alex Waygood <[email protected]>
AlexWaygood
AlexWaygood reviewed Aug 29, 2023
View reviewed changes
Comment thread Doc/howto/clinic.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@vstinner @AlexWaygood
Update Doc/howto/clinic.rst
60ec4ec 
Co-authored-by: Alex Waygood <[email protected]>
@vstinner

vstinner commented Aug 29, 2023

Copy link
Copy Markdown
Member Author

You could left it undocumented if only I use it. 😄

I prefer to document it. Maybe I can add a note: "option reserved to Serhiy's private usage" 🤣
AlexWaygood and erlend-aasland reacted with laugh emoji
All reactions
  • 😄 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AlexWaygood
AlexWaygood approved these changes Aug 29, 2023
View reviewed changes
@AlexWaygood AlexWaygood changed the title gh-108494: Argument Clinic: Automatically determine if the Limited API is used gh-108494: Argument Clinic: Document how to generate code that uses the limited C API Aug 29, 2023
@bedevere-bot bedevere-bot removed the awaiting core review label Aug 29, 2023
@bedevere-bot bedevere-bot added the awaiting merge label Aug 29, 2023
@AlexWaygood

AlexWaygood commented Aug 29, 2023

Copy link
Copy Markdown
Member

I'm not sure that this PR title. Usage of the Limited C API is already determined automatically in the main branch, I did it in a previous change. This PR is more about documenting my recent AC work.

I changed it again ;)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Sure, without the code changes, and only the documentation change remaining, it makes sense to title it as a documentation change ;)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland
erlend-aasland approved these changes Aug 29, 2023
View reviewed changes
erlend-aasland
erlend-aasland reviewed Aug 29, 2023
View reviewed changes
Comment thread Doc/howto/clinic.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland
erlend-aasland reviewed Aug 29, 2023
View reviewed changes
Comment thread Misc/NEWS.d/next/Tools-Demos/2023-08-25-22-40-12.gh-issue-108494.4RbDdu.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
vstinner and others added 2 commits August 29, 2023 22:51
@vstinner @erlend-aasland
Update Doc/howto/clinic.rst
84687de 
Co-authored-by: Erlend E. Aasland <[email protected]>
@vstinner @erlend-aasland
Update Misc/NEWS.d/next/Tools-Demos/2023-08-25-22-40-12.gh-issue-1084…
214f6bf 
…94.4RbDdu.rst

Co-authored-by: Erlend E. Aasland <[email protected]>
@vstinner
vstinner merged commit 2928e5d into python:main Aug 30, 2023
@vstinner
vstinner deleted the clinic_limited_doc branch August 30, 2023 14:02
@bedevere-bot bedevere-bot removed the awaiting merge label Aug 30, 2023
@vstinner

vstinner commented Aug 30, 2023

Copy link
Copy Markdown
Member Author

Merged, thanks.
AlexWaygood and erlend-aasland reacted with hooray emoji
All reactions
  • 🎉 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
carljm added a commit to carljm/cpython that referenced this pull request Aug 30, 2023
@carljm
Merge branch 'main' into icompexc
37377e6 
* main:
  pythongh-108520: Fix bad fork detection in nested multiprocessing use case (python#108568)
  pythongh-108590: Revert pythongh-108657 (commit 400a1ce) (python#108686)
  pythongh-108494: Argument Clinic: Document how to generate code that uses the limited C API (python#108584)
  Document Python build requirements (python#108646)
  pythongh-101100: Fix Sphinx warnings in the Logging Cookbook (python#108678)
  Fix typo in multiprocessing docs (python#108666)
  pythongh-108669: unittest: Fix documentation for TestResult.collectedDurations (python#108670)
  pythongh-108590: Fix sqlite3.iterdump for invalid Unicode in TEXT columns (python#108657)
  Revert "pythongh-103224: Use the realpath of the Python executable in `test_venv` (pythonGH-103243)" (pythonGH-108667)
  pythongh-106320: Remove private _Py_ForgetReference() (python#108664)
  Mention Ellipsis pickling in the docs (python#103660)
  Revert "Use non alternate name for Kyiv (pythonGH-108533)" (pythonGH-108649)
  pythongh-108278: Deprecate passing the first param of sqlite3.Connection callback APIs by keyword (python#108632)
  pythongh-108455: peg_generator: install two stubs packages before running mypy (python#108637)
  pythongh-107801: Improve the accuracy of io.IOBase.seek docs (python#108268)
AA-Turner pushed a commit to AA-Turner/devguide that referenced this pull request Sep 26, 2023
@vstinner @AlexWaygood @erlend-aasland
python/cpython#108494: Argument Clinic: Document how to generate code…
2e3b673 
… that uses the limited C API (python/cpython#108584)

Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
erlend-aasland added a commit to python/devguide that referenced this pull request Oct 1, 2023
@vstinner @AlexWaygood @erlend-aasland
python/cpython#108494: Argument Clinic: Document how to generate code…
0128018 
… that uses the limited C API (python/cpython#108584)

Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes
@AlexWaygood AlexWaygood AlexWaygood approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

skip news topic-argument-clinic

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 @erlend-aasland @AlexWaygood @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.