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

Use python_requires to force pip to not download if on python2 #20

Closed
mpacer opened this issue Oct 6, 2017 · 6 comments · Fixed by #21
Closed

Use python_requires to force pip to not download if on python2 #20

mpacer opened this issue Oct 6, 2017 · 6 comments · Fixed by #21

Comments

@mpacer
Copy link
Collaborator

mpacer commented Oct 6, 2017

You need to use python_requires in your setup.py in order to restrict builds that occur via pip which issues a query to PyPI.

Below is a long explanation of what's happening and what you need to do, but the tldr is:

I installed allofplos on python2 with no difficulty, that's not supposed to happen.

What's wrong

I understand based on

if sys.version_info.major < 3:
    sys.exit('Sorry, Python < 3.4 is not supported')
elif sys.version_info.minor < 4:
    sys.exit('Sorry, Python < 3.4 is not supported')

That installing on python 2 (or even 3.3) is not supposed to be possible.

Unfortunately, it is. E.g., I installed allofplos on python 2:

(dev2) ~/jupyter/eg_notebooks $ python --version
Python 2.7.13 :: Continuum Analytics, Inc.
(dev2) ~/jupyter/eg_notebooks $ pip --version
pip 9.0.1 from ~/anaconda3/envs/dev2/lib/python2.7/site-packages (python 2.7)
(dev2) ~/jupyter/eg_notebooks $ pip install allofplos
Collecting allofplos
  Downloading allofplos-0.8.1-py2.py3-none-any.whl
⋮
Successfully installed allofplos-0.8.1 certifi-2017.7.27.1 chardet-3.0.4 idna-2.6 lxml-4.0.0 progressbar2-3.34.3 python-utils-2.2.0 requests-2.18.4 tqdm-4.17.1 urllib3-1.22

A PR is forthcoming, specifically to add python_requires >= 3.4 to your setup(…) args & fix a couple of other things.

Why python_requires works

When the package is being pulled down from PyPI if someone has pip>=9 then it will check the python version before it downloads the file and if their version does not not match python_requires.

Why your current approach fails

Currently your only approach doesn't work for a subtle reason: it only catches people who run python setup.py directly or who have a really old version of pip. As mentioned in the talk linked below, everyone should be discouraged from running python setup.py directly, but you've got the right thought to try to catch those that do catches for those who do).

This also means you need another catch inside the setup.py in case someone tried to download it with pip but their pip version is way less than 9.0.0.

This is how we handle the pip version problem in IPython's setup.py:

if sys.version_info < (3, 3):
    pip_message = 'This may be due to an out of date pip. Make sure you have pip >= 9.0.1.'
    try:
        import pip
        pip_version = tuple([int(x) for x in pip.__version__.split('.')[:3]])
        if pip_version < (9, 0, 1) :
            pip_message = 'Your pip version is out of date, please install pip >= 9.0.1. '\
            'pip {} detected.'.format(pip.__version__)
        else:
            # pip is new enough - it must be something else
            pip_message = ''
    except Exception:
        pass

This also requires that anyone building the package from source has a setuptools >= 24.2.0. If you don't want to make that a requirement on the actual library, then you'll need a different way to communicate that dependency (possibly with a dev_requirements.txt and a CONTRIBUTING.md?).

Resources for learning more

I mentioned this to @eseiver, but in case she didn't pass it along there's a great talk on this here: https://www.youtube.com/watch?v=2DkfPzWWC2Q. 😉

If you'd prefer to read the docs though:

https://packaging.python.org/tutorials/distributing-packages/#python-requires

@sbassi
Copy link
Collaborator

sbassi commented Oct 6, 2017

Thank you for the explanation!
Will check it tomorrow.

@sbassi sbassi closed this as completed in #21 Oct 6, 2017
@mpacer
Copy link
Collaborator Author

mpacer commented Oct 6, 2017

You should probably issue a patch release with this ASAP, that way the python2 issue won't arise for others.

@sbassi
Copy link
Collaborator

sbassi commented Oct 6, 2017

I thought it was covered with your patch that has this line python_requires='>=3.4',

@mpacer
Copy link
Collaborator Author

mpacer commented Oct 6, 2017

Yes, but you need to release 0.8.1 with #21 for that to take effect.

@mpacer
Copy link
Collaborator Author

mpacer commented Oct 6, 2017

Also weirdly… warehouse has a pypi 0.8.1
https://pypi.org/project/allofplos/
image

but pypi legacy is still pointing to 0.8.0:

https://pypi.python.org/pypi/allofplos
image

Even though it has a 0.8.1 branch.
image

And that 0.8.1 branch lacks a data-requires-python field on it's target:

image

Example of what it should look like:

image

Did you rebuild the package with a setuptools >=24?

Also, it looks like you didn't distribute an sdist (which you should probably do for downstream packaging folks).

@sbassi
Copy link
Collaborator

sbassi commented Oct 9, 2017

This is ver setuptools I have:

setuptools.version
'27.2.0'

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