Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support __del__ #2479

Open
samuelcolvin opened this issue Jun 25, 2022 · 2 comments · May be fixed by #2484
Open

Support __del__ #2479

samuelcolvin opened this issue Jun 25, 2022 · 2 comments · May be fixed by #2484

Comments

@samuelcolvin
Copy link
Contributor

I'd like to be able to use a __del__ method in #[pymethods], however it's not being called.

There are two other related issue #402 and #774. #774 seems most relevant but the OP get's told they shouldn't be using __del__. AFAIK issuing a ResourceWarning is a legitimate use of __del__.

I could print a warning in Drop but that won't be a python warning and will confuse people, without support for __del__ I don't see another way around this.

Related PR samuelcolvin/watchfiles#164

@davidhewitt
Copy link
Member

Sounds reasonable to add this!

Some notes for implementer (probably me eventually, unless someone else is interested in picking this up). I think the underlying C API "slot" is tp_finalize. See implementation used for Python classes at https://github.com/python/cpython/blob/4e08fbcfdfa57ea94091aabdd09413708e3fb2bf/Objects/typeobject.c#L8033

It looks like the tp_dealloc in that file also calls tp_finalize in some circumstances; we might need to check carefully that we reproduce the Python behaviour correctly.

@samuelcolvin
Copy link
Contributor Author

Great, thank you.

No particular hurry from me, it's a possibly overkill warning in a semi-private class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants