GeezerVison

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

GeezerVison is showing github.com — gh-128509: Add `sys._is_immortal` for identifying immortal objects by ZeroIntensity · Pull Request #128510 · python/cpython

Conversation

ZeroIntensity commented Jan 5, 2025 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

FWIW, I'm basing most of this off sys._is_interned.

Issue: Add an API for determining if an object is immortal #128509

📚 Documentation preview 📚: https://cpython-previews--128510.org.readthedocs.build/en/128510/library/sys.html#sys._is_immortal

Sorry, something went wrong.

Uh oh!

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

eendebakpt, dg-pb, and johnslavik reacted with thumbs up emoji

All reactions

👍 3 reactions

ZeroIntensity added 7 commits

January 4, 2025 20:01


          Add sys._is_immortal

120bc73


          Add documentation note.

ac45261


          Add a note to the docs and fix tests.

b545f24


          Add some documentation notes.

49b0ea8


          Add a note in the glossary.

b31efe5


          Change up wording.

45dc9e0


          Add blur

32d7db0

ZeroIntensity requested a review from ericsnowcurrently as a code owner

January 5, 2025 01:56

bedevere-app Bot mentioned this pull request

Jan 5, 2025

Add an API for determining if an object is immortal #128509

Closed

bedevere-app Bot added the awaiting review label

Jan 5, 2025


          Merge branch 'main' into is-immortal-sys

8bb4f62

ZeroIntensity mentioned this pull request

Jan 22, 2025

gh-128509: Add PyUnstable_IsImmortal for finding immortal objects #129182

Merged

ZeroIntensity added 3 commits

January 27, 2025 10:13


          Remove redundant test case.

638edd0


          Merge branch 'main' of https://github.com/python/cpython into is-immo…

20bb910

…rtal-sys


          Test for sys._is_immortal in the existing case.

64c9a52

ZeroIntensity requested a review from vstinner

January 27, 2025 15:17

ZeroIntensity commented Jan 27, 2025

Copy link

Copy Markdown

Member Author

@vstinner I think this is ready now that #129182 is merged. Could you take a look when you get a chance?

All reactions

Sorry, something went wrong.

Uh oh!

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


          Add note about the C API.

7e416e7

vstinner reviewed

Jan 27, 2025

View reviewed changes

Comment thread

Lib/test/test_capi/test_immortal.py Outdated

		# CRASHES _testcapi.is_immortal(NULL)

		def test_sys(self):
		self.immortal_checking(sys._is_immortal)

vstinner Jan 27, 2025

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.

sys functions must be checked in test_sys.

Sorry, something went wrong.

Uh oh!

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

All reactions

ZeroIntensity Jan 27, 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 intentionally moved the test from test_sys to test_immortal so we could re-use the test case, but I've reverted it now. Are you ok with the redundancy?

Sorry, something went wrong.

Uh oh!

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

All reactions

vstinner Jan 27, 2025

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.

I'm ok with redundancy. test_capi can be skipped if _testcapi is missing. It's not the case for test_sys. Both tests are useful.

Sorry, something went wrong.

Uh oh!

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

ZeroIntensity reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

Doc/library/sys.rst Resolved

Uh oh!

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

Comment thread

Doc/library/sys.rst Resolved

Uh oh!

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

Comment thread

Python/sysmodule.c Outdated

		sys__is_immortal_impl(PyObject module, PyObject op)
		/[clinic end generated code: output=c2f5d6a80efb8d1a input=83733fc356c78475]/
		{
		return _Py_IsImmortal(op);

vstinner Jan 27, 2025

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.

Why not using PyUnstable_IsImmortal()?

Sorry, something went wrong.

Uh oh!

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

All reactions

ZeroIntensity Jan 27, 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.

The two are pretty similar. Would you prefer I use the unstable API?

Sorry, something went wrong.

Uh oh!

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

All reactions

vstinner Jan 27, 2025

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.

I would prefer to use the public unstable API, yes.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Python/sysmodule.c Resolved

Uh oh!

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

ZeroIntensity added 4 commits

January 27, 2025 10:30


          Revert "Test for sys._is_immortal in the existing case."

10560ef

This reverts commit 64c9a52.


          Align to 80 characters.

51daa10


          Move _is_immortal to after _is_interned

fdcafec


          Switch to the unstable API

380bca7

vstinner approved these changes

Jan 27, 2025

View reviewed changes

vstinner 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.

LGTM. IMO it makes sense to have a way in Python to check if an object is immortal. Otherwise, developers are confused by surprising sys.getrefcount() values.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Python/sysmodule.c

		}
		}

vstinner Jan 27, 2025

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.

unrelated change, I suggest to leave this empty line :-)

Sorry, something went wrong.

Uh oh!

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

All reactions

ZeroIntensity Jan 27, 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.

Good catch!

Sorry, something went wrong.

Uh oh!

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

All reactions

bedevere-app Bot added awaiting merge and removed awaiting review labels

Jan 27, 2025


          Fix stray newline change

74a751a

vstinner commented Jan 28, 2025

Copy link

Copy Markdown

Member

cc @colesbury @kumaraditya303

All reactions

Sorry, something went wrong.

Uh oh!

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

kumaraditya303 reviewed

Jan 28, 2025

View reviewed changes

Comment thread

Python/sysmodule.c Resolved

Uh oh!

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


          Update sysmodule.c

be49c26

Co-authored-by: Kumar Aditya <[email protected]>


          Rerun clinic.

1154cfe

ZeroIntensity requested a review from kumaraditya303

January 29, 2025 15:22

kumaraditya303 approved these changes

Jan 29, 2025

View reviewed changes


          Merge branch 'main' into is-immortal-sys

a071b86

kumaraditya303 enabled auto-merge (squash)

January 31, 2025 15:04

kumaraditya303 merged commit 9ba281d into python:main

Jan 31, 2025

bedevere-app Bot removed the awaiting merge label

Jan 31, 2025

ZeroIntensity deleted the is-immortal-sys branch

January 31, 2025 16:16

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request

Feb 7, 2025


          pythongh-128509: Add sys._is_immortal for identifying immortal obje…

6a8d880

…cts (python#128510)

Co-authored-by: Kumar Aditya <[email protected]>

ZeroIntensity mentioned this pull request

Oct 27, 2025

Add type stub for sys._is_immortal python/typeshed#14931

Merged

Reviewers

vstinner

vstinner approved these changes

kumaraditya303

kumaraditya303 approved these changes

ericsnowcurrently

Awaiting requested review from ericsnowcurrently

ericsnowcurrently 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.

3 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 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.

Conversation

ZeroIntensity commented Jan 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZeroIntensity commented Jan 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner commented Jan 28, 2025

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ZeroIntensity commented Jan 5, 2025 •

edited

Loading

Uh oh!

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