GeezerVison

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

GeezerVison is showing github.com — gh-103091: Add PyUnstable_Type_AssignVersionTag by swtaarrs · Pull Request #103095 · python/cpython

Conversation

swtaarrs commented Mar 28, 2023

Copy link

Copy Markdown

Member

Add a new C-API function to eagerly assign a version tag to a PyTypeObject. Details in gh-103091.

Sorry, something went wrong.

Uh oh!

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

All reactions


          pythongh-103091: Add PyType_AssignVersionTag

3b329c6

swtaarrs requested a review from markshannon as a code owner

March 28, 2023 21:30

bedevere-bot mentioned this pull request

Mar 28, 2023

Add a C API to assign a new version to a PyTypeObject #103091

Closed

bedevere-bot added the awaiting review label

Mar 28, 2023

ghost commented Mar 28, 2023 •

edited by ghost

Loading

Uh oh!

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

Copy link

Copy Markdown

All commit authors signed the Contributor License Agreement.

All reactions

Sorry, something went wrong.

Uh oh!

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

encukou reviewed

Mar 31, 2023

View reviewed changes

Comment thread

Include/object.h Outdated

		# define PyType_CheckExact(op) PyType_CheckExact(_PyObject_CAST(op))
		#endif

		#if !defined(Py_LIMITED_API)

encukou Mar 31, 2023

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.

You can put the definition in Include/cpython/object.h, rather than use this #ifdef here. (See line 825.)

Sorry, something went wrong.

Uh oh!

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

carljm and swtaarrs reacted with thumbs up emoji

All reactions

👍 2 reactions

carljm Mar 31, 2023

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.

See https://devguide.python.org/developer-workflow/c-api/index.html for details on this.

Sorry, something went wrong.

Uh oh!

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

All reactions

carljm requested changes

Mar 31, 2023

View reviewed changes

carljm left a comment

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.

Requesting changes for renaming to PyUnstable_Type_AssignVersionTag and moving to Include/cpython/object.h.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Include/object.h Outdated

		# define PyType_CheckExact(op) PyType_CheckExact(_PyObject_CAST(op))
		#endif

		#if !defined(Py_LIMITED_API)

carljm Mar 31, 2023

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.

See https://devguide.python.org/developer-workflow/c-api/index.html for details on this.

Sorry, something went wrong.

Uh oh!

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

All reactions

bedevere-bot added awaiting changes and removed awaiting review labels

Mar 31, 2023

bedevere-bot commented Mar 31, 2023

Copy link

Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

All reactions

Sorry, something went wrong.

Uh oh!

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

This comment was marked as off-topic.


          Rename to PyUnstable_AssignVersionTag, move to Include/cptyhon/object…

840f6c9

….h, fix some newlines

swtaarrs commented Mar 31, 2023

Copy link

Copy Markdown

Member Author

Thanks for the comments!

I have made the requested changes; please review again.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-bot added awaiting change review and removed awaiting changes labels

Mar 31, 2023

bedevere-bot commented Mar 31, 2023

Copy link

Copy Markdown

Thanks for making the requested changes!

@carljm: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-bot requested a review from carljm

March 31, 2023 18:23

carljm approved these changes

Mar 31, 2023

View reviewed changes

carljm left a comment

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 looks good to me. @markshannon any concerns here?

Sorry, something went wrong.

Uh oh!

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

All reactions

bedevere-bot added awaiting merge and removed awaiting change review labels

Mar 31, 2023

carljm requested changes

Apr 6, 2023

View reviewed changes

carljm left a comment

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.

On second look here, I realized the updated name isn't quite what was requested; it should be PyUnstable_Type_AssignVersionTag (not just PyUnstable_AssignVersionTag).

Sorry, something went wrong.

Uh oh!

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

All reactions

bedevere-bot added awaiting changes and removed awaiting merge labels

Apr 6, 2023

bedevere-bot commented Apr 6, 2023

Copy link

Copy Markdown

All reactions

Sorry, something went wrong.

Uh oh!

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

swtaarrs added 2 commits

April 20, 2023 09:06


          Rename to PyUnstable_Type_AssignVersionTag

b838340


          Merge remote-tracking branch 'upstream/main' into type-version-tag

e971ceb

swtaarrs changed the title ~~gh-103091: Add PyType_AssignVersionTag~~ gh-103091: Add PyUnstable_Type_AssignVersionTag

Apr 20, 2023

swtaarrs commented Apr 20, 2023

Copy link

Copy Markdown

Member Author

Sorry about that! I fixed the name and merged latest main.

I have made the requested changes; please review again.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-bot added awaiting change review and removed awaiting changes labels

Apr 20, 2023

bedevere-bot commented Apr 20, 2023

Copy link

Copy Markdown

Thanks for making the requested changes!

@carljm: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-bot requested a review from carljm

April 20, 2023 16:15

carljm approved these changes

Apr 22, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels

Apr 22, 2023

carljm commented Apr 24, 2023

Copy link

Copy Markdown

Member

Checked this in person with @markshannon and @ericsnowcurrently at the PyCon sprints; going ahead and merging.

All reactions

Sorry, something went wrong.

Uh oh!

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

carljm merged commit b7f4811 into python:main

Apr 24, 2023

bedevere-bot removed the awaiting merge label

Apr 24, 2023

carljm added a commit to carljm/cpython that referenced this pull request

Apr 24, 2023


          Merge branch 'main' into superopt

19b8025

* main: (53 commits)
  pythongh-102498 Clean up unused variables and imports in the email module  (python#102482)
  pythongh-99184: Bypass instance attribute access in `repr` of `weakref.ref` (python#99244)
  pythongh-99032: datetime docs: Encoding is no longer relevant (python#93365)
  pythongh-94300: Update datetime.strptime documentation (python#95318)
  pythongh-103776: Remove explicit uses of $(SHELL) from Makefile (pythonGH-103778)
  pythongh-87092: fix a few cases of incorrect error handling in compiler (python#103456)
  pythonGH-103727: Avoid advancing tokenizer too far in f-string mode (pythonGH-103775)
  Revert "Add tests for empty range equality (python#103751)" (python#103770)
  pythongh-94518: Port 23-argument `_posixsubprocess.fork_exec` to Argument Clinic (python#94519)
  pythonGH-65022: Fix description of copyreg.pickle function (python#102656)
  pythongh-103323: Get the "Current" Thread State from a Thread-Local Variable (pythongh-103324)
  pythongh-91687: modernize dataclass example typing (python#103773)
  pythongh-103746: Test `types.UnionType` and `Literal` types together (python#103747)
  pythongh-103765: Fix 'Warning: py:class reference target not found: ModuleSpec' (pythonGH-103769)
  pythongh-87452: Improve the Popen.returncode docs
  Removed unnecessary escaping of asterisks (python#103714)
  pythonGH-102973: Slim down Fedora packages in the dev container (python#103283)
  pythongh-103091: Add PyUnstable_Type_AssignVersionTag (python#103095)
  Add tests for empty range equality (python#103751)
  pythongh-103712: Increase the length of the type name in AttributeError messages (python#103713)
  ...

carljm added a commit to carljm/cpython that referenced this pull request

Apr 24, 2023


          Merge branch 'superopt' into superopt_spec

1972ca4

* superopt: (82 commits)
  pythongh-101517: fix line number propagation in code generated for except* (python#103550)
  pythongh-103780: Use patch instead of mock in asyncio unix events test (python#103782)
  pythongh-102498 Clean up unused variables and imports in the email module  (python#102482)
  pythongh-99184: Bypass instance attribute access in `repr` of `weakref.ref` (python#99244)
  pythongh-99032: datetime docs: Encoding is no longer relevant (python#93365)
  pythongh-94300: Update datetime.strptime documentation (python#95318)
  pythongh-103776: Remove explicit uses of $(SHELL) from Makefile (pythonGH-103778)
  pythongh-87092: fix a few cases of incorrect error handling in compiler (python#103456)
  pythonGH-103727: Avoid advancing tokenizer too far in f-string mode (pythonGH-103775)
  Revert "Add tests for empty range equality (python#103751)" (python#103770)
  pythongh-94518: Port 23-argument `_posixsubprocess.fork_exec` to Argument Clinic (python#94519)
  pythonGH-65022: Fix description of copyreg.pickle function (python#102656)
  pythongh-103323: Get the "Current" Thread State from a Thread-Local Variable (pythongh-103324)
  pythongh-91687: modernize dataclass example typing (python#103773)
  pythongh-103746: Test `types.UnionType` and `Literal` types together (python#103747)
  pythongh-103765: Fix 'Warning: py:class reference target not found: ModuleSpec' (pythonGH-103769)
  pythongh-87452: Improve the Popen.returncode docs
  Removed unnecessary escaping of asterisks (python#103714)
  pythonGH-102973: Slim down Fedora packages in the dev container (python#103283)
  pythongh-103091: Add PyUnstable_Type_AssignVersionTag (python#103095)
  ...

swtaarrs deleted the type-version-tag branch

January 16, 2024 22:14

Reviewers

encukou

encukou left review comments

carljm

carljm approved these changes

markshannon

Awaiting requested review from markshannon

markshannon is a code owner

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

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.

GeezerVison

Conversation

swtaarrs commented Mar 28, 2023

Uh oh!

ghost commented Mar 28, 2023 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

encukou Mar 31, 2023

Choose a reason for hiding this comment

Uh oh!

carljm Mar 31, 2023

Choose a reason for hiding this comment

Uh oh!

carljm left a comment

Choose a reason for hiding this comment

Uh oh!

carljm Mar 31, 2023

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Mar 31, 2023

Uh oh!

This comment was marked as off-topic.

swtaarrs commented Mar 31, 2023

Uh oh!

bedevere-bot commented Mar 31, 2023

Uh oh!

carljm left a comment

Choose a reason for hiding this comment

Uh oh!

carljm left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Apr 6, 2023

Uh oh!

swtaarrs commented Apr 20, 2023

Uh oh!

bedevere-bot commented Apr 20, 2023

Uh oh!

carljm commented Apr 24, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ghost commented Mar 28, 2023 •

edited by ghost

Loading

Uh oh!

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