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

DEP: Add 'python_requires' to setup.py to drop 3.4 support #20698

Merged
merged 3 commits into from Apr 16, 2018

Conversation

Projects
None yet
4 participants
@djhoese
Contributor

djhoese commented Apr 14, 2018

  • closes #20697
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

This is a small change to the setup.py that should stop newer versions of pandas from being downloaded in environments with older/unsupported versions of python. I followed the instructions here: https://packaging.python.org/guides/dropping-older-python-versions/

setup.py Outdated
@@ -748,4 +748,5 @@ def pxd(name):
long_description=LONG_DESCRIPTION,
classifiers=CLASSIFIERS,
platforms='any',
python_requires='>2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*',

This comment has been minimized.

@jreback

jreback Apr 15, 2018

Contributor

hmm, there has to be a better way to write this, to signify only 2.7 or >=3.5

This comment has been minimized.

@djhoese

djhoese Apr 15, 2018

Contributor

I was following numpy (https://github.com/numpy/numpy/blob/master/setup.py#L368) and the link I mentioned.

This comment has been minimized.

@djhoese

djhoese Apr 15, 2018

Contributor

I agree that it should be simpler but wasn't sure what conditionals were allowed.

This comment has been minimized.

@djhoese

djhoese Apr 15, 2018

Contributor

Oh, I do notice a big mistake. It should be >=2.7 not >2.7. Researching to see if there is a shorter way.

This comment has been minimized.

@djhoese

djhoese Apr 15, 2018

Contributor
In [15]: from pip.utils.packaging import specifiers, version

In [16]: version.parse('3.5.0') in specifiers.SpecifierSet('==2.7,>=3.5.*')
Out[16]: False

This comment has been minimized.

@jreback

jreback Apr 15, 2018

Contributor

I think you can do >=3.5 ?

@jreback

This comment has been minimized.

Contributor

jreback commented Apr 15, 2018

@jreback jreback added the Build label Apr 15, 2018

@codecov

This comment has been minimized.

codecov bot commented Apr 15, 2018

Codecov Report

Merging #20698 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #20698      +/-   ##
==========================================
+ Coverage   91.81%   91.81%   +<.01%     
==========================================
  Files         153      153              
  Lines       49275    49279       +4     
==========================================
+ Hits        45241    45245       +4     
  Misses       4034     4034
Flag Coverage Δ
#multiple 90.2% <ø> (ø) ⬆️
#single 41.9% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/frame.py 97.16% <0%> (ø) ⬆️
pandas/core/indexes/multi.py 95.06% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec7968d...24a9270. Read the comment docs.

@djhoese

This comment has been minimized.

Contributor

djhoese commented Apr 15, 2018

Moving the discussion outside of the commits in case more changes are done. Simple test for a simpler python_requires:

In [15]: from pip.utils.packaging import specifiers, version

In [16]: version.parse('3.5.0') in specifiers.SpecifierSet('==2.7,>=3.5.*')
Out[16]: False

# later I did:
In [2]: version.parse('3.5.0') in specifiers.SpecifierSet('==2.7,>=3.5')
Out[2]: False

@djhoese

This comment has been minimized.

Contributor

djhoese commented Apr 15, 2018

I read the source for SpecifierSet and Specifier. The condition is basically all(my_version in specifier for specifier in specifiers) where the __contains__ is doing the individual condition for each version specifier (my_version == 2.7 and my_version >=3.5).

I guess you'll just have to drop python 2.7 sooner /s

Source: https://github.com/pypa/pip/blob/master/src/pip/_vendor/packaging/specifiers.py#L180
https://github.com/pypa/pip/blob/master/src/pip/_vendor/packaging/specifiers.py#L724

@jreback

This comment has been minimized.

Contributor

jreback commented Apr 15, 2018

@davidh-ssec ok this looks ok. can you change our min for setuptools to 24.2.0, see here (which is pretty old anyhow). and update the Dependencies in install.rst & in the whatsnew.

@jreback jreback added this to the 0.23.0 milestone Apr 15, 2018

@djhoese

This comment has been minimized.

Contributor

djhoese commented Apr 15, 2018

@jreback You want the setuptools version in setup_requires? Or is there somewhere else it is currently specified?

@djhoese

This comment has been minimized.

Contributor

djhoese commented Apr 15, 2018

Oh you meant set the min version in install.rst and update the deps in the 0.23 whatsnew. For the deps do you want it added to the min version table of deps?

@jreback

This comment has been minimized.

Contributor

jreback commented Apr 15, 2018

yes + in

(pandas) bash-3.2$ grep setuptools ci/*
ci/environment-dev.yaml:  - setuptools>=3.3
ci/requirements_dev.txt:setuptools>=3.3

(just update these)

@jreback jreback merged commit 8756f55 into pandas-dev:master Apr 16, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jreback

This comment has been minimized.

Contributor

jreback commented Apr 16, 2018

thanks @davidh-ssec

@TomAugspurger

This comment has been minimized.

Contributor

TomAugspurger commented Apr 16, 2018

@jorisvandenbossche

This comment has been minimized.

Member

jorisvandenbossche commented Apr 17, 2018

Should we remove our pyproject.toml alltogether for upcoming 0.23.0 until support in pip is more mature? It seems that also the varying numpy version depending on the python version is not yet supported (cfr scipy/scipy#8734)

casperdcl added a commit to tqdm/tqdm that referenced this pull request Apr 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment