Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-134819: Add sys.set_object_tags and sys.get_object_tags#135073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
corona10 wants to merge13 commits intopython:main
base:main
Choose a base branch
Loading
fromcorona10:gh-134819

Conversation

corona10
Copy link
Member

@corona10corona10 commentedJun 3, 2025
edited by bedevere-appbot
Loading

@corona10corona10 changed the titlegh-134819: Add sys.set_object_tags and sys.get_object_tags[WIP] gh-134819: Add sys.set_object_tags and sys.get_object_tagsJun 3, 2025
@corona10corona10 changed the title[WIP] gh-134819: Add sys.set_object_tags and sys.get_object_tagsgh-134819: Add sys.set_object_tags and sys.get_object_tagsJun 3, 2025
@corona10
Copy link
MemberAuthor

@vstinner@colesbury@Fidget-Spinner@ZeroIntensity
Before making more progress, including documentation stuff.
I would like to get feedback on the current implementation.

{
assert(object != NULL);
if (strcmp(tag, "immortal") == 0) {
_Py_SetImmortal(object);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This definitely isn't safe on its own. (Trust me, I've gone downquite the rabbithole in getting arbitrary object immortalization working. It's extraordinarily complex to do safely.)

Copy link
MemberAuthor

@corona10corona10Jun 3, 2025
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Hmm, do you think that we should not allow setting "immortal" tag at this moment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yeah, at least for now.sys.set_object_tags is allowed to ignore tags, right?

Copy link
MemberAuthor

@corona10corona10Jun 3, 2025
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yeah, we will ignore (and it's intended behavior)

@vstinner
Copy link
Member

Before making more progress, including documentation stuff. I would like to get feedback on the current implementation.

As I wrote in the issue, I dislike this API and I prefer the current design: one function per object attribute, such assys._is_immortal().

@corona10
Copy link
MemberAuthor

corona10 commentedJun 3, 2025
edited
Loading

As I wrote in the issue, I dislike this API and I prefer the current design: one function per object attribute, such as sys._is_immortal()

The problem is that the alternative implementation also needs to provide the same functions to pass the unittests and avoid people from checking attributes based on the CPython-only function. So this API will remove unnecessary additional APIs that are only for checking the CPython implementation detail.

I also dislike that we are exposing CPython implementation detail to the pure Python function.
(C API is fine.. It's highly intertwined with CPython.)

@corona10
Copy link
MemberAuthor

So if you think that we're fine with exposing CPython implementation details through Python and C API. I can happily drop this proposal.

@colesbury
Copy link
Contributor

I agree with@vstinner here and still feel the same way as inmy comment on the issue. I am also not in a rush to expose these APIs at the Python level.

corona10 reacted with eyes emoji

@ZeroIntensity
Copy link
Member

I'm indifferent. I see arguments for both sides that both make sense.

In general, I think we do need a way to expose unstable APIs to Python the same way we do in C. It's definitely not fun to have to set up a C extension just so you can get something that is useful at a Python level (e.g., deferred reference counting). I think Donghee makes a good point that prefixing with_ isn't a sufficient way to do that. Rather than getting hung up on how object implementation details will be exposed, I think we should focus on the bigger picture. How should we expose implementation details to Python, regardless of objects in specific?

corona10 reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ZeroIntensityZeroIntensityZeroIntensity left review comments

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

@vstinnervstinnerAwaiting requested review from vstinner

@colesburycolesburyAwaiting requested review from colesbury

@Fidget-SpinnerFidget-SpinnerAwaiting requested review from Fidget-Spinner

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@corona10@vstinner@colesbury@ZeroIntensity

[8]ページ先頭

©2009-2025 Movatter.jp