-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Allow install under pypy #2559
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
Allow install under pypy #2559
Conversation
PyPy raises an IndexError unpickling bad pickle files.
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Both unit-tests and mypy fail for pypy3.6. As such, it's likely that far too many changes would be needed to justify supporting the older version.
The older ast does not support quite a few unicode edge cases and is disliked by mypy.
So I have run tox and mypy locally with pypy3.7 all passes except the python2 parsing. It would probably take a lot of changes to fix the failures for pypy3.6. Since that wouldn't really be in the spirit of this PR I removed it from the list of targets. It doesn't make sense to support parsing python2 syntax using pypy3 (seems like too much of a niche use case). |
We have to pass information about the interpreter to tox, so that we can skip the python2 tests
Previously in CI, pypy tests would hang forever due to https://foss.heptapod.net/pypy/pypy/-/issues/3317. This was fixed in the latest release, so we can remove the workaround.
This reverts commit 6c66ab3. Even though this bug is not present for newer versions of pypy, the github action tends to receive older versions (7.3.5 on macos and 7.3.6 on windows/linux)
This is now ready, modulo any reviewer's thoughts. Most of the work was actually for the CI pipeline and not real code changes, but I have it passing over on my repo https://github.com/olliemath/black/actions/runs/1394800461 PyPy 3.8 works fine too - but it's still in beta - so I've parked it over here olliemath#3 pending a stable release |
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.
Thanks! I'm happy to add support for pypy, but have a few comments.
Hi @JelleZijlstra thanks for the review. I tightened up the FAQ based on your comments (although I couldn't apply your suggestion, due to the file being changed). I'm not sure that I understand the CI failures - something to do with the hypothesis lib not being formatted correctly? I can't see how that would relate to my changes. Anyhow, let me know if further changes are needed. |
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.
The CI failure seems unrelated.
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.
Given that we'll be dropping Python 2 support in 2-3 months I don't think we'll need to do much in terms of letting folks know PyPy doesn't support Python 2. Perhaps we should notice when our built-in parser (which has excellent cross-version syntax support; ignoring parenthesized with and pattern matching -> #2586) falls back to the Python 2 grammar and emit a friendly error but we can deal with that later as it isn't strictly related to PyPy compatibility.
Hmm, to be fair usually it's good to test out development versions of Python implementations so we can root out issues early, but I don't have any experience with PyPy so I'll delegate the decision to someone who knows better ^^ |
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com>
Description
This adds a minimal set of changes to use black under pypy. It's supposed to be a more acceptable version of #1172 with the key point being there are zero changes for cpython users. The hope is that this will enough to close #727
This is a draft - feedback appreciated - I plan to do all of the following (if needed):
Checklist - did you ...