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-121654: Add PyType_Freeze() function by vstinner · Pull Request #122457 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Jul 30, 2024 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • Issue: Limited API way of assigning to immutable types and/or making a class immutable #121654

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
vstinner requested review from a team, encukou and markshannon as code owners July 30, 2024 12:49
@bedevere-app bedevere-app Bot mentioned this pull request Jul 30, 2024
Limited API way of assigning to immutable types and/or making a class immutable #121654
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label Jul 30, 2024
@vstinner

vstinner commented Jul 30, 2024

Copy link
Copy Markdown
Member Author

cc @encukou @da-woods
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@da-woods

da-woods commented Jul 30, 2024

Copy link
Copy Markdown
Contributor

Looks sensible to me.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
encukou
encukou reviewed Aug 1, 2024
View reviewed changes

@encukou encukou 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.

It'll take me some time to review this in depth, unfortunately.
Here are some initial thoughts for what I'd want to check.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/c-api/type.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/typeobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
vstinner added 5 commits August 26, 2024 17:31
@vstinner
pythongh-121654: Add PyType_Freeze() function
2d24aba
@vstinner
Call PyType_Modified()
f7bd310
@vstinner
Fix typo
f7d75ac
@vstinner
Add more elaborate test on MRO
1fcda68
@vstinner
Check __mro__ instead of __bases__
beea5e3
@vstinner
vstinner force-pushed the type_freeze branch from 9c8de19 to beea5e3 Compare August 26, 2024 15:32
@vstinner

vstinner commented Aug 26, 2024

Copy link
Copy Markdown
Member Author

@encukou: Please review my updated PR. I modified my PR to check __mro__ instead of __bases__.

I also rebased the PR on the main branch to fix some merge conflicts.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
encukou
encukou reviewed Aug 27, 2024
View reviewed changes

@encukou encukou 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.

Looks good, thank you!
I have a few suggestions, and the C API WG should vote on new stable API, but I hope those are just formalities.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/c-api/type.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/c-api/type.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_capi/test_type.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
vstinner and others added 2 commits August 27, 2024 14:13
@vstinner @encukou
Update Doc/c-api/type.rst
0155362 
Co-authored-by: Petr Viktorin <[email protected]>
@vstinner @encukou
Update Lib/test/test_capi/test_type.py
1c1fa7e 
Co-authored-by: Petr Viktorin <[email protected]>
@vstinner vstinner mentioned this pull request Aug 27, 2024
Add PyType_Freeze() function capi-workgroup/decisions#40
Closed
@vstinner

vstinner commented Aug 27, 2024

Copy link
Copy Markdown
Member Author

the C API WG should vote on new stable API, but I hope those are just formalities.

I created capi-workgroup/decisions#40
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Type must not be used before made immutable
e0cc4be
@vstinner

vstinner commented Sep 23, 2024

Copy link
Copy Markdown
Member Author

@zooba: I added to the doc:

The type must not be used before it's made immutable. For example, type
instances must not be created before the type is made immutable.

Is it what you expected?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Merge branch 'main' into type_freeze
a9f1f01
@vstinner

vstinner commented Sep 30, 2024

Copy link
Copy Markdown
Member Author

@encukou @serhiy-storchaka: Would you mind to review my PyType_Freeze() implementation?
All reactions

Sorry, something went wrong.

Uh oh!

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

vstinner commented Sep 30, 2024

Copy link
Copy Markdown
Member Author

See also PR gh-124789 implementing @zooba's Py_tp_create_callback slot idea.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Merge branch 'main' into type_freeze
0f98b64
@vstinner

vstinner commented Oct 17, 2024

Copy link
Copy Markdown
Member Author

@encukou @serhiy-storchaka @erlend-aasland @zooba: Would you mind to review this PR? The API was approved by the C API Working Group.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka approved these changes Oct 19, 2024
View reviewed changes
Comment thread Lib/test/test_capi/test_type.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Objects/typeobject.c
Comment on lines +11342 to +11346
if (!PyTuple_Check(mro)) {
Py_DECREF(mro);
PyErr_SetString(PyExc_TypeError, "unable to get the type MRO");
return -1;
}

@serhiy-storchaka serhiy-storchaka Oct 19, 2024

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.

How difficult to add test for this case?

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner Oct 19, 2024

Copy link
Copy Markdown
Member Author

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 not sure that it's possible to create a type with a MRO which is not a tuple. Using a metaclass, it's possible to override the MRO, but internally, Python converts mro() result into a tuple.

I added this check since type_get_mro() returns None if type->tp_mro is NULL. But I have no idea how to create a type with type->tp_mro=NULL. It's more a sanity check.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Oct 19, 2024
@vstinner
Test the error message
2752daa
erlend-aasland
erlend-aasland approved these changes Oct 21, 2024
View reviewed changes
@vstinner
vstinner merged commit db96327 into python:main Oct 25, 2024
@vstinner
vstinner deleted the type_freeze branch October 25, 2024 09:12
@bedevere-app bedevere-app Bot removed the awaiting merge label Oct 25, 2024
@vstinner

vstinner commented Oct 25, 2024

Copy link
Copy Markdown
Member Author

Merged. Thanks for reviews.
erlend-aasland reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@da-woods

da-woods commented Oct 26, 2024

Copy link
Copy Markdown
Contributor

Thanks Victor. I'll look at using this after the next CPython release
erlend-aasland and vstinner reacted with heart emoji
All reactions
  • ❤️ 2 reactions

Sorry, something went wrong.

Uh oh!

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

AraHaan commented Oct 30, 2024

Copy link
Copy Markdown
Contributor

I mean technically you can use it now if you manually build the main branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@vstinner @encukou @ebonnal
pythongh-121654: Add PyType_Freeze() function (python#122457)
533a40a 
Co-authored-by: Petr Viktorin <[email protected]>
sergey-miryanov
sergey-miryanov reviewed May 13, 2025
View reviewed changes
Comment thread Objects/typeobject.c
i = 1;
}
for (; i<PyTuple_GET_SIZE(bases); i++) {
PyTypeObject *b = (PyTypeObject*)PyTuple_GET_ITEM(bases, i);

@sergey-miryanov sergey-miryanov May 13, 2025

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.

Sorry for the necro-posting.
IIUC only improperly constructed tuples can store NULL items. Is this such a case? If so, is the following check redundant?

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner May 16, 2025

Copy link
Copy Markdown
Member Author

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 don't know, I just moved the code. The test was already there before.

Sorry, something went wrong.

Uh oh!

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

@sergey-miryanov sergey-miryanov May 16, 2025

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.

Got it, thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sergey-miryanov sergey-miryanov sergey-miryanov left review comments
@encukou encukou encukou left review comments
@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@erlend-aasland erlend-aasland erlend-aasland approved these changes
@markshannon 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.

7 participants

@vstinner @da-woods @AraHaan @sergey-miryanov @encukou @serhiy-storchaka @erlend-aasland
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 01402520 — 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.