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

C implementation of ZoneInfo cannot be subclassed #82

Closed
sdispater opened this issue Jun 18, 2020 · 2 comments · Fixed by #83
Closed

C implementation of ZoneInfo cannot be subclassed #82

sdispater opened this issue Jun 18, 2020 · 2 comments · Fixed by #83

Comments

@sdispater
Copy link

While working on supporting zoneinfo in pendulum, I got this error whenever I subclassed Zoneinfo:

TypeError: descriptor '__init_subclass__' of 'backports.zoneinfo.ZoneInfo' object needs an argument

When using the zoneinfo module available in Python 3.9, I got this error:

TypeError: unbound method ZoneInfo.__init_subclass__() needs an argument

Note that there is no issue with the pure Python implementation.

So, I went digging and it appears that, at the C level, the __init_subclass__ / zoneinfo_init_subclass must be declared as a class method via METH_CLASS here: https://github.com/pganssle/zoneinfo/blob/master/lib/zoneinfo_module.c#L2584

I tested this quickly and it seems to fix the issue.

The fix should be applied on the backport and in the standard library.

@pganssle
Copy link
Owner

Huh... I can reproduce this with:

from backports.zoneinfo import ZoneInfo

class ZoneInfoSubclass(ZoneInfo):
    pass

But what I don't understand is why this test is not affected.

I guess whatever is wrong is making it so the C __init_subclass__ is never getting called, since that's never hit in the C coverage.

@pganssle
Copy link
Owner

Ah, I see the problem with the test. It's not inheriting from ZoneInfoSubclassTest. 😅

Fix coming up.

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 a pull request may close this issue.

2 participants