Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
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
base:main
Are you sure you want to change the base?
Conversation
@vstinner@colesbury@Fidget-Spinner@ZeroIntensity |
Python/sysmodule.c Outdated
{ | ||
assert(object != NULL); | ||
if (strcmp(tag, "immortal") == 0) { | ||
_Py_SetImmortal(object); |
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 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.)
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.
Hmm, do you think that we should not allow setting "immortal" tag at this moment?
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.
Yeah, at least for now.sys.set_object_tags
is allowed to ignore tags, right?
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.
Yeah, we will ignore (and it's intended behavior)
Uh oh!
There was an error while loading.Please reload this page.
As I wrote in the issue, I dislike this API and I prefer the current design: one function per object attribute, such as |
corona10 commentedJun 3, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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. |
So if you think that we're fine with exposing CPython implementation details through Python and C API. I can happily drop this proposal. |
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. |
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 |
Uh oh!
There was an error while loading.Please reload this page.