Skip to content

BLD: Require enum34 for Python < 3.4 #24153

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

Closed
wants to merge 2 commits into from

Conversation

effigies
Copy link

@effigies effigies commented Dec 7, 2018

#22802 imports enum, which was added to the built-ins in Python 3.4. The enum34 package backports enum to earlier Python versions, so this should be a dependency as long as Python 2.7 is still supported.

To demonstrate the issue, creating a minimal environment using conda and installing the nightly Pandas build suffices:

conda create -y -n pandas_nightly python=2.7
conda activate pandas_nightly
pip install --pre --find-links=https://5cf40426d9f06eb7461d-6fe47d9331aba7cd62fc36c7196769e4.ssl.cf2.rackcdn.com --find-links https://7933911d6844c6c53a7d-47bd50c35cd79bd838daf386af554a83.ssl.cf2.rackcdn.com pandas

Then:

$ python -c 'import pandas'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/anaconda3/envs/pandas_nightly/lib/python2.7/site-packages/pandas/__init__.py", line 35, in <module>
    "the C extensions first.".format(module))
ImportError: C extension: No module named enum not built. If you want to import pandas from the source directory, you may need to run 'python setup.py build_ext --inplace --force' to build the C extensions first.

If I install enum34:

$ pip install enum34
Collecting enum34
  Using cached https://files.pythonhosted.org/packages/c5/db/e56e6b4bbac7c4a06de1c50de6fe1ef3810018ae11732a50f15f62c7d050/enum34-1.1.6-py2-none-any.whl
Installing collected packages: enum34
Successfully installed enum34-1.1.6
$ python -c 'import pandas'
$
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

Not sure what the best way to test this would be. Currently, installing hypothesis masks this error, because it contains the requirement enum34; python_version == "2.7".

Related: MacPython/pandas-wheels#36, bids-standard/pybids#276, bids-standard/pybids#315

pandas-dev#22802 imports `enum`, which was added to the built-ins in Python 3.4. The `enum34` package backports `enum` to earlier Python versions, so this should be a dependency as long as Python 2.7 is still supported.
@pep8speaks
Copy link

Hello @effigies! Thanks for submitting the PR.

  • There are no PEP8 issues in the file setup.py !

@datapythonista datapythonista added the Dependencies Required and optional dependencies label Dec 7, 2018
@codecov
Copy link

codecov bot commented Dec 7, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24153      +/-   ##
==========================================
- Coverage    92.2%    92.2%   -0.01%     
==========================================
  Files         162      162              
  Lines       51700    51700              
==========================================
- Hits        47671    47670       -1     
- Misses       4029     4030       +1
Flag Coverage Δ
#multiple 90.6% <ø> (-0.01%) ⬇️
#single 43.02% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/internals/blocks.py 93.65% <0%> (-0.07%) ⬇️

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 67faf6c...f5e0e15. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Dec 7, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24153      +/-   ##
==========================================
- Coverage    92.2%    92.2%   -0.01%     
==========================================
  Files         162      162              
  Lines       51700    51700              
==========================================
- Hits        47671    47670       -1     
- Misses       4029     4030       +1
Flag Coverage Δ
#multiple 90.6% <ø> (-0.01%) ⬇️
#single 43.02% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/internals/blocks.py 93.65% <0%> (-0.07%) ⬇️

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 67faf6c...f5e0e15. Read the comment docs.

@datapythonista
Copy link
Member

I guess this makes sense, even if after the next version only Python>=3.5 will be supported, so it'll be only for 0.24.

@TomAugspurger

@jreback
Copy link
Contributor

jreback commented Dec 7, 2018

we may actually want to just change it and not use enums which is slightly simpler

adding a required dependency has some implications for wheel and conda building (meaning we haven’t touched these in a while)

@TomAugspurger
Copy link
Contributor

Yeah we should be able to write this without an enum fairly easily.

@jreback
Copy link
Contributor

jreback commented Dec 9, 2018

superseded by #24170

@jreback jreback closed this Dec 9, 2018
@effigies effigies deleted the patch-1 branch December 9, 2018 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Required and optional dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants