Skip to content

Conversation

StanFromIreland
Copy link
Member

@StanFromIreland StanFromIreland commented Oct 14, 2025

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

ZLIB_CRC32_COMBINE_METHODDEF
ZLIB_DECOMPRESS_METHODDEF
ZLIB_DECOMPRESSOBJ_METHODDEF
{"__getattr__", zlib_getattr, METH_VARARGS, "Module __getattr__"},
Copy link
Member

Choose a reason for hiding this comment

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

I tried to convert the function to Argument Clinic, but it failed, oh!

Error in file 'Modules/zlibmodule.c' on line 2019:
'__getattr__' is a special method and cannot be converted to Argument Clinic!

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that too :-)

Copy link
Member

Choose a reason for hiding this comment

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

Would this be a valid feature request for AC or it’s somehow impossible?

Copy link
Member Author

@StanFromIreland StanFromIreland Oct 19, 2025

Choose a reason for hiding this comment

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

I tried, and it is relatively straightforward (I based my implementation on METHOD_INIT's), here is a patch. I do not know if there was a reason for this not being done before, other than there not being a need, as it is explicitly in the unsupported list, maybe Victor knows more?

Copy link
Member

Choose a reason for hiding this comment

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

I have no idea why it's not supported currently.

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you think about the patch?

Copy link
Member

Choose a reason for hiding this comment

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

You can propose it as a PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened #140376

@vstinner vstinner merged commit 46f11b3 into python:main Oct 15, 2025
49 checks passed
@vstinner
Copy link
Member

Merged, thanks. I also saw that version but forgot to deprecate it :-)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants