Update python3 note in README

msull commented 6 years ago

README has outdated information about being able to install this package in python3.

https://github.com/agronholm/pythonfutures/pull/67

agronholm commented 6 years ago

Please explain the reasons for the change. How is installing futures bad now? I just tried and the standard library is still preferred over the backport.

msull commented 6 years ago

The change in the linked PR (https://github.com/agronholm/pythonfutures/pull/67) makes it so futures 3.2.0 and above cannot install on python3, rather than should not be. I was just updating the README to reflect that reality. One of my users reported this yesterday:

$ python --version
Python 3.5.2
$ pip install futures
Collecting futures
futures requires Python '>=2.6, <3' but the running Python is 3.5.2

It's weird actually-- I guess the exact behavior will depend on the version of pip, I cannot recreate this right now, when I try with python3 and latest pip version 3.2.0 futures isn't even seen as an option.

agronholm commented 6 years ago

Ah, right, I forgot about that totally. But I would like to strongly discourage people from even trying to install it on Python 3, and that has not changed since the beginning. The text seems a tad too elaborate with that in mind.

I'm curious – how did your user end up complaining about this?

msull commented 6 years ago

https://github.com/datarobot/batch-scoring/blob/36e7c992f98e5f3fd3f8c89bca0d774efb48da2e/requirements.txt#L5

We have python2/python3 package that is lists futures as a requirement, and a user reported this problem yesterday while installing.

We will be updating to use a python2 conditional requirement for our futures pin, but I thought I would update the README here as well since I found it a bit confusing when investigating the issue.

agronholm commented 6 years ago

Alright. By the way, your setup looks far more complicated than it should be. For example, why do you have a separate requirements.txt? Why not put the dependencies in either setup.py or setup.cfg?

msull commented 6 years ago

Yes I agree with that assessment. This is actually my first interaction with this particular repository in our company, so I can't really say why it is like this currently, but I imagine we will do some cleanup when resolving this issue.

As for this PR, if you'd prefer to not add this feel free to close it out, or if you can suggest a wording you prefer I will update.

Thanks!

homeworkprod commented 6 years ago

The README currently states:

It should not be installed on Python 3, although there should be no harm in doing so, as the standard library takes precedence over third party libraries.

However, futures 3.2.0 breaks tests for me:

/builds/project/project/.eggs/freezegun-0.3.10-py3.5.egg/freezegun/_async.py:3: in <module>
    ???
/usr/local/lib/python3.5/asyncio/__init__.py:21: in <module>
    from .base_events import *
/usr/local/lib/python3.5/asyncio/base_events.py:17: in <module>
    import concurrent.futures
.eggs/futures-3.2.0-py3.5.egg/concurrent/futures/__init__.py:8: in <module>
    from concurrent.futures._base import (FIRST_COMPLETED,
E     File "/builds/project/project/.eggs/futures-3.2.0-py3.5.egg/concurrent/futures/_base.py", line 414
E       raise exception_type, self._exception, self._traceback
E                           ^
E   SyntaxError: invalid syntax

Neither freezegun nor its dependencies python-dateutil and six seem to depend on futures.

However, my project depends on Flask-MarrowMailer which does unconditionally depend on futures (maybe as a result of the README stating that it does no harm).

As long as futures 3.1.1 is installed, things seem to work fine, but with futures 3.2.0 they break as in the traceback above – and also make noise at install time:

Searching for python-dateutil
Reading https://pypi.python.org/simple/python-dateutil/
Downloading https://pypi.python.org/packages/54/bb/f1db86504f7a49e1d9b9301531181b00a1c7325dc85a29160ee3eaa73a54/python-dateutil-2.6.1.tar.gz#md5=db38f6b4511cefd76014745bb0cc45a4
Best match: python-dateutil 2.6.1
Processing python-dateutil-2.6.1.tar.gz
Writing /tmp/easy_install-8rz0bikj/python-dateutil-2.6.1/setup.cfg
Running python-dateutil-2.6.1/setup.py -q bdist_egg --dist-dir /tmp/easy_install-8rz0bikj/python-dateutil-2.6.1/egg-dist-tmp-y31radxa
  File "build/bdist.linux-x86_64/egg/concurrent/futures/_base.py", line 414
    raise exception_type, self._exception, self._traceback
                        ^
SyntaxError: invalid syntax

  File "/builds/yolas/yolas/.eggs/futures-3.2.0-py3.5.egg/concurrent/futures/_base.py", line 414
    raise exception_type, self._exception, self._traceback
                        ^
SyntaxError: invalid syntax

agronholm commented 6 years ago

How are you installing this so that import concurrent.futures actually imports the backport instead of the standard library module? That should not be possible without explicit manipulation of sys.path.

homeworkprod commented 6 years ago

I'm still figuring this one out.

It's just the python:3.5 Docker image with virtualenv and pip installed on top of it, then installing the project's requirements via pip. Test runner is pytest's, triggered via python setup.py test.

homeworkprod commented 6 years ago

Maybe it is related to the way pytest imports things.

Here is a complete error log for a failing test:

============================= test session starts ==============================
platform linux -- Python 3.5.5, pytest-3.4.2, py-1.5.2, pluggy-0.6.0
rootdir: /builds/project/project, inifile: setup.cfg
collected 371 items / 6 errors

==================================== ERRORS ====================================
___ ERROR collecting tests/test_something.py ____
.eggs/pytest-3.4.2-py3.5.egg/_pytest/python.py:411: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
.eggs/py-1.5.2-py3.5.egg/py/_path/local.py:668: in pyimport
    __import__(modname)
<frozen importlib._bootstrap>:968: in _find_and_load
    ???
<frozen importlib._bootstrap>:957: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:664: in _load_unlocked
    ???
<frozen importlib._bootstrap>:634: in _load_backward_compatible
    ???
.eggs/pytest-3.4.2-py3.5.egg/_pytest/assertion/rewrite.py:213: in load_module
    py.builtin.exec_(co, mod.__dict__)
tests/test_something.py:10: in <module>
    from freezegun import freeze_time
<frozen importlib._bootstrap>:968: in _find_and_load
    ???
<frozen importlib._bootstrap>:957: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:664: in _load_unlocked
    ???
<frozen importlib._bootstrap>:634: in _load_backward_compatible
    ???
/builds/project/project/.eggs/freezegun-0.3.10-py3.5.egg/freezegun/__init__.py:9: in <module>
    ???
<frozen importlib._bootstrap>:968: in _find_and_load
    ???
<frozen importlib._bootstrap>:957: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:664: in _load_unlocked
    ???
<frozen importlib._bootstrap>:634: in _load_backward_compatible
    ???
/builds/project/project/.eggs/freezegun-0.3.10-py3.5.egg/freezegun/api.py:45: in <module>
    ???
<frozen importlib._bootstrap>:968: in _find_and_load
    ???
<frozen importlib._bootstrap>:957: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:664: in _load_unlocked
    ???
<frozen importlib._bootstrap>:634: in _load_backward_compatible
    ???
/builds/project/project/.eggs/freezegun-0.3.10-py3.5.egg/freezegun/_async.py:3: in <module>
    ???
/usr/local/lib/python3.5/asyncio/__init__.py:21: in <module>
    from .base_events import *
/usr/local/lib/python3.5/asyncio/base_events.py:17: in <module>
    import concurrent.futures
.eggs/futures-3.2.0-py3.5.egg/concurrent/futures/__init__.py:8: in <module>
    from concurrent.futures._base import (FIRST_COMPLETED,
E     File "/builds/project/project/.eggs/futures-3.2.0-py3.5.egg/concurrent/futures/_base.py", line 414
E       raise exception_type, self._exception, self._traceback
E                           ^
E   SyntaxError: invalid syntax

agronholm commented 6 years ago

How do you even end up with an .eggs directory in the first place? Dependencies are supposed to be installed to site-packages in the virtualenv.

homeworkprod commented 6 years ago

That's probably the result of python setup.py test and loading all lines from requirements.txt in the setup.py file, assigning them as install_requires dependencies.

I think tox creates .eggs/, too, but while it gets installed (I should probably change that), it is not used.

homeworkprod commented 6 years ago

For the record: The actual issue of futures (>= 3.2.0) being installed on Python 3 has been solved for me with the release of Flask-MarrowMailer 0.3.0 which no longer includes futures as a dependency on Python 3.

May the above tracebacks help others to track down related issues they might have.

Extra thanks to @agronholm for investigating.

One thing remains: The "installing on Python 3 does no harm" line in the README should definitely be changed (as this example shows).

agronholm commented 6 years ago

I still want to understand thoroughly how the issue is caused in the first place before doing anything.

ikalnytskyi commented 6 years ago

Alright, so here's what's happened.

Since futures 3.2, the package has python_requires='>=2.6, <3', check in setup.py.
That basically means, you may not be able to install this project in python 3 environments, because setuptools may not allow you to do so.
This will depends on setuptools version you have installed in your system. Newest versions will raise error if you try to install futures in python enviroment that doesn't meet constraint python_requires='>=2.6, <3',.

agronholm commented 6 years ago

I know all of this. But none of it explains how 1) a user was still able to install it despite the python_requires AND the manual Python version check in setup.py and 2) it was installed in a way that somehow overrides the standard library package at import time.

ikalnytskyi commented 6 years ago

1) a user was still able to install it despite the python_requires

Easy. Please try to install it on Ubuntu Xenial. It comes with setuptools 20.7.0 that does not evaluate this check.

2) it was installed in a way that somehow overrides the standard library package at import time.

I have no idea. In my experience, it does not.

The only thing I can add is that

it's pain now to maintain python wheels, because we can't use universal wheel anymore and must build two wheels: one for py2 (that specifies futures in install_requires) and one for py3 (that does not).
everything got broken around the word, because people used to rely on futures package that used to work on both py2 and py3.

agronholm commented 6 years ago

I have no idea. In my experience, it does not.

If that were true, nobody would be having issues. Try unpacking the futures wheel in site-packages on Python 3.X and then import the concurrent.futures package.

From what I've seen around these problem reports recently is that people still use python setup.py test which installs dependencies as eggs instead of wheels, using setuptools instead of pip. The whole test command is ill conceived and only supports unittest style test suites without hacks. If you can confirm that this reproduces the problem, I will update the README accordingly.

agronholm / pythonfutures

Update python3 note in README #71