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

[MRG+1] Mention PyPy support, add PyPy to install docs #3048

Merged
merged 1 commit into from Dec 25, 2017

Conversation

@lopuhin
Copy link
Member

@lopuhin lopuhin commented Dec 25, 2017

As discussed in #2213, mention that we have PyPy support, and describe installation issues. I put PyPy section at the end, along with platform issues, although it's not strictly a platform.

I discovered one issue though that I'll try to resolve in another PR: right now pypy-specific dependency PyPyDispatcher is never picked up. A way to check and a workaround is also in the docs, but not everyone reads them so I'd like to try fixing this before release.

@lopuhin lopuhin requested a review from kmike Dec 25, 2017
@@ -69,9 +69,11 @@ Here's an example spider using BeautifulSoup API, with ``lxml`` as the HTML pars
What Python versions does Scrapy support?
-----------------------------------------

Scrapy is supported under Python 2.7 and Python 3.4+.
Scrapy is supported under Python 2.7 and Python 3.4+
under CPython (default Python implementation) and PyPy (only for Python 2.7).
Copy link
Member

@kmike kmike Dec 25, 2017

Choose a reason for hiding this comment

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

Do you know what's the reason pypy3 is not supported? Is it just us not running tests for it, or are there things which work in pypy2, but not in pypy3?

Copy link
Member Author

@lopuhin lopuhin Dec 25, 2017

Choose a reason for hiding this comment

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

Last time I checked, lxml did not install on PyPy 3. I filed an issue about it and it was quickly fixed, but then it stopped working again. I didn't check in a while though, let me check again.

@kmike kmike changed the title Mention PyPy support, add PyPy to install docs [MRG+1] Mention PyPy support, add PyPy to install docs Dec 25, 2017
kmike
kmike approved these changes Dec 25, 2017
Copy link
Member

@kmike kmike left a comment

LGTM 👍 It'd be good to fix PyPyDispatcher and add pypy3 support, but this PR describes current state of pypy support nicely, so +1 to merge.

@codecov
Copy link

@codecov codecov bot commented Dec 25, 2017

Codecov Report

Merging #3048 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3048   +/-   ##
=======================================
  Coverage   84.51%   84.51%           
=======================================
  Files         164      164           
  Lines        9270     9270           
  Branches     1380     1380           
=======================================
  Hits         7835     7835           
  Misses       1177     1177           
  Partials      258      258

@lopuhin
Copy link
Member Author

@lopuhin lopuhin commented Dec 25, 2017

Turned out that scrapy 1.4.0 didn't have that pypy-specific dependency yet, and if I build a wheel from master and install it locally, then PyPyDispatcher is picked up correctly - hopefully this will be the same for the wheel for 1.5.0 (I checked that py27 build installs setuptools-38.2.5 which is more than enough for this feature to work).

@lopuhin lopuhin added this to the v1.5 milestone Dec 25, 2017
If this command gives errors such as
``TypeError: ... got 2 unexpected keyword arguments``, this means
that setuptools was unable to pick up one PyPy-specific dependency.
To fix this issue, run ``pip install 'PyPyDispatcher>=2.1.0'``.
Copy link
Member

@kmike kmike Dec 25, 2017

Choose a reason for hiding this comment

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

@lopuhin do you prefer to keep this note, just in case, or to remove it?

Copy link
Member Author

@lopuhin lopuhin Dec 25, 2017

Choose a reason for hiding this comment

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

I'd rather keep it just in case. In theory pypy comes with a recent enough setuptools, but I remember that someone still had this issue somehow (although that could be with 1.4.0 which I thought already had this feature). If we don't get any reports about this, we can remove it in the next release.

@kmike kmike merged commit 9f9edea into scrapy:master Dec 25, 2017
2 checks passed
@kmike
Copy link
Member

@kmike kmike commented Dec 25, 2017

Looks good! Let's tackle pypy3 separately.

@lopuhin lopuhin deleted the pypy-install-docs branch Dec 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants