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

Allow install under pypy #2559

Merged
merged 25 commits into from Nov 14, 2021
Merged

Allow install under pypy #2559

merged 25 commits into from Nov 14, 2021

Conversation

olliemath
Copy link
Contributor

@olliemath olliemath commented Oct 22, 2021

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 ...

  • Add a CHANGELOG entry if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

olliemath and others added 3 commits October 22, 2021 14:27
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.
@olliemath
Copy link
Contributor Author

olliemath commented Oct 22, 2021

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).
So I think a nice error message if under pypy and parsing a py2 file and some way to inform tox not to run the python2 tests under pypy would be the next step.

We have to pass information about the interpreter to tox, so that we can skip the python2 tests
@olliemath olliemath marked this pull request as draft October 22, 2021 17:13
src/black/parsing.py Outdated Show resolved Hide resolved
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.
@olliemath olliemath marked this pull request as ready for review October 27, 2021 10:35
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)
@olliemath
Copy link
Contributor Author

olliemath commented Oct 28, 2021

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

Copy link
Collaborator

@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! I'm happy to add support for pypy, but have a few comments.

docs/faq.md Outdated Show resolved Hide resolved
docs/faq.md Outdated Show resolved Hide resolved
@olliemath
Copy link
Contributor Author

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.

Copy link
Collaborator

@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.

The CI failure seems unrelated.

Copy link
Collaborator

@ichard26 ichard26 left a 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.

@ichard26
Copy link
Collaborator

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

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 ^^

CHANGES.md Outdated Show resolved Hide resolved
@JelleZijlstra JelleZijlstra merged commit eb9d039 into psf:main Nov 14, 2021
JelleZijlstra added a commit that referenced this pull request Nov 16, 2021
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com>
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.

Support PyPy (easy)
4 participants