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

Restore compatibility with PyPy <3.9 #262

Merged
merged 2 commits into from
Jul 1, 2023

Conversation

AlexWaygood
Copy link
Member

Fixes #258.

It looks like the problem is that PyPy on <3.9 looks up __mro_entries__ on the class rather than the instance. That's the correct behaviour for 99% of dunders, but __mro_entries__ is extremely special in that it needs to be looked up on the instance rather than the class; it's this quality that allows us to monkey-patch __mro_entries__ methods onto functions.

It also looks like some of our tests have been broken on PyPy <3.9 for quite a while -- the NamedTuple function-based implementation also didn't work on PyPy 3.8.

Comment on lines +3010 to +3011
and __tp is not Protocol
and __tp is not getattr(typing, "Protocol", object())
Copy link
Member Author

Choose a reason for hiding this comment

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

is_protocol(typing.Protocol) was evaluating to True on PyPy-3.8. I don't know why that is, really, but this change fixes it.

Copy link
Member

Choose a reason for hiding this comment

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

I think I used != on purpose here so that Protocol can have a custom __eq__ that helps us out. What you're doing here should work too, though.

Copy link
Member Author

@AlexWaygood AlexWaygood Jul 1, 2023

Choose a reason for hiding this comment

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

Yeah, I have no idea why _ProtocolMeta.__eq__ doesn't seem to be doing what it should be doing on PyPy 3.8. I suspect another bug in PyPy. It's easy enough for us to work around it here, though :)

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks!

This looks good, but Matti's response in https://foss.heptapod.net/pypy/pypy/-/issues/3958#note_299926 makes me think that maybe we should drop support for pypy3.7/3.8 instead, so let's wait to see what he responds.

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Jul 1, 2023

This looks good, but Matti's response in https://foss.heptapod.net/pypy/pypy/-/issues/3958#note_299926 makes me think that maybe we should drop support for pypy3.7/3.8 instead, so let's wait to see what he responds.

Yeah... I guess the question is whether we're okay with mandating that black, trio, pydantic, cattrs and pylint all drop support for PyPy-3.7 and PyPy-3.8 as well? Seems like quite a few projects that depend on typing_extensions still test with PyPy-3.8 in CI ://

@JelleZijlstra
Copy link
Member

Matti suggested we should drop support for pypy3.7/3.8. However, given that this is a regression and so many downstream packages still depend on pypy3.7/3.8, I'm inclined to merge this and release 4.7.1, then in 4.8.0 formally drop support for pypy3.7/3.8.

We can also ask downstream packages to drop support. I know at least one maintainer of Black will agree.

@AlexWaygood
Copy link
Member Author

That sounds like a reasonable plan.

My personal opinion -- I'm fully on board with dropping support for PyPy-3.7, since Python 3.7 is EOL. For PyPy-3.8, before dropping support, I'd prefer to first have a conversation with packages who depend on us and who we know test with PyPy-3.8 in CI. As you say, since PyPy-3.8 is no longer supported, maybe they shouldn't be testing with it in CI anymore -- but for packages that we know we'd be breaking, I'd still prefer to have the conversation first, I think.

And whatever the case, I agree that it's a good idea to restore compatibility in a patch release now, then consider what to do in the next feature release after that.

I know at least one maintainer of Black will agree.

👀

@JelleZijlstra JelleZijlstra merged commit dcdc53f into python:main Jul 1, 2023
@AlexWaygood AlexWaygood deleted the fix-pypy branch July 1, 2023 19:45
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.

4.7.0: new implementation of TypedDict breaks pypy-3.7 and pypy-3.8
2 participants