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

Consider depending on setuptools in setup.py #1897

Closed
cel4 opened this issue Aug 17, 2014 · 25 comments

Comments

Projects
None yet
4 participants
@cel4
Copy link

commented Aug 17, 2014

Please consider making setuptools a mandatory dependency for installing statsmodels.

The current setup.py is hard to read and especially the invocation of the dependency checking part is very hackish and possibly the reason for this issue:
https://bitbucket.org/hpk42/tox/issue/184/tox-fails-to-install-statsmodels-due-to

With mandatory setuptools all dependencies could be handled by install_requires and extras_require which would reduce code and would make setup.py much cleaner.

If you insist on using distutils as a fallback, please consider calling check_dependency_versions only if setuptools is not present and handle dependencies in a setuptools conform way otherwise.

@josef-pkt

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

For now there will not be any dependency information that pip can discover in statsmodels, as long as tox/pip upgrades all dependencies by default. see for example #1267

However, @rgommers has a work around to create a requires.txt for scipy when there is no importable numpy, that is when installing into an empty virtualenv.
I haven't looked at the details, and we don't have it yet for statsmodels.

@josef-pkt josef-pkt added build and removed build labels Aug 17, 2014

@cel4

This comment has been minimized.

Copy link
Author

commented Aug 17, 2014

thanks for your input @josef-pkt, I skimmed through the issue and I think it reports are very similar issue. Though I do not understand why this crash occurs, yet.
Besides that, think it really makes sense to rewrite the setup.py and use a setuptools only build system with a list of dependencies.

@josef-pkt

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

(as I said before in many different places)
I don't want tox/pip to update my dependencies, definitely not numpy and scipy, and currently I don't see a way out if we define the dependencies/requirements in a way that pip will understand.

http://stackoverflow.com/questions/15280896/how-to-prevent-tox-from-deleting-installed-packages

@josef-pkt

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

I don't know the details, but AFAIU:
The current situation is that it works if the user defines his/her own requires.txt with all dependencies.

@cel4

This comment has been minimized.

Copy link
Author

commented Aug 17, 2014

@josef-pkt,

I don't want tox/pip to update my dependencies, definitely not numpy and scipy

If I understand you correctly, the issue is that scipy takes quite a long time to compile and you would prefer installing a binary instead? This makes sense, but I don't see how that relates to tox. If I recall correctly, tox only recreates the virtual environments if you change the dependencies. Do you test by manually updating tox.ini to the dependency version you would like to test?

@rgommers

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

@cel4 no, the main issue is that pip upgrades not only statsmodels but also dependencies by default. Upgrading numpy should be an explicit choice by the user, not a side product of pip install statsmodels or pip install -U statsmodels, because there's a lot that can go wrong (broken installs, version incompatibilities for other packages with numpy as a depency, etc). pip devs have agreed that the behavior of pip install -U is bad, but so far not changed it. Hence manual dependency checking it is.

That said, this situation could be improved by a similar change to scipy/scipy@8e7ee0c. @josef-pkt if you agree, I can submit a PR for that.

@josef-pkt

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

I'm on Windows and I'm not compiling scipy or matplotlib, and numpy only for testing. (Actually, I'm not set up at all anymore to compile scipy).

The main point is that numpy is a "system package" that many other packages depend on. We never ever want to automatically update numpy. I don't think you would want pip to update your python to the latest py 3.4 or py 3.5 version.

Second, pip is stupid on Windows and I avoid it as much as possible. It uninstalls numpy and scipy, but is not able to reinstall it, so it leaves me with a broken python installation.

Third, all my tox virtualenv are carefully build by hand with manual installation of the binaries. It has exactly the version combinations of the statsmodels dependencies that I want.

In some cases tox is not able to find a binary installed numpy and wipes it anyway, and tries unsuccessfully to install a new version.

After spending several afternoons recreating my tox virtualenvs that tox/pip wiped, this became my favorite "pet-hate". (and I don't have many of those.)

(This happened some time ago, and I haven't updated my tox and pip in a while. I might upgrade when wheels are in common distribution, and I have a day or two to fix the consequences of pip mistakes.)

@josef-pkt

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

@rgommers Yes, please open a PR with that change.
AFAICS, it would solve the problem for users that want to install into an empty virtualenv.

Did it work without problems for scipy?

@rgommers

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

Yes, no issues for ~4 months.

For OS X there are now numpy/scipy wheels, which solves most of the install failures. Hopefully we'll manage to get Windows wheels as well soon.

@cel4

This comment has been minimized.

Copy link
Author

commented Aug 17, 2014

Okay, I think I got the problem now. Thanks for clarification.

@josef-pkt

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

@rgommers Thanks

rgommers added a commit to rgommers/statsmodels that referenced this issue Aug 17, 2014

BLD: let setuptools/pip handle dependencies that are not installed at…
… all.

Closes statsmodelsgh-1897.  See scipy commit 8e7ee0c4b for a similar change that has
worked well.
@rgommers

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

Done in gh-1902. @cel4 please check that it does fix your issue.

@cel4

This comment has been minimized.

Copy link
Author

commented Aug 17, 2014

@rgommers, @josef-pkt: I am having troubles testing this fix. It seems as if I'm running into another issue:

Installing collected packages: statsmodels
  Running setup.py install for statsmodels
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/test/.tox/py27/build/statsmodels/setup.py", line 395, in <module>
        write_version_py()
      File "/tmp/test/.tox/py27/build/statsmodels/setup.py", line 223, in write_version_py
        FULLVERSION += '.dev-' + GIT_REVISION[:7]
    UnboundLocalError: local variable 'GIT_REVISION' referenced before assignment

My first impression is that it should be if not ISRELEASED and dowrite: in line 222 https://github.com/rgommers/statsmodels/blob/build-deps/setup.py#L222

@rgommers

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

Indeed an unrelated issue. I think the fix is to put GIT_REVISION = "Unknown" also in the elif clause at line 218. Your fix should also get you past that point.

@rgommers

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

I'll add that fix to the PR.

rgommers added a commit to rgommers/statsmodels that referenced this issue Aug 17, 2014

@cel4

This comment has been minimized.

Copy link
Author

commented Aug 17, 2014

Running tox now works, but listing statsmodels in the dependency section will only pull in statsmodels in an emtpy environment. tox does not seem to pull in the dependencies.

@rgommers

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

If the dependencies already are installed in the tox env, then they indeed shouldn't be reinstalled or upgraded. That was exactly what my PR intended to do. But maybe I'm missing your point. Can you elaborate a bit (error message or commands to reproduce your remaining issue)?

@cel4

This comment has been minimized.

Copy link
Author

commented Aug 17, 2014

This is my tox.ini

[tox]
envlist = py27
[testenv]
deps =
  statsmodels
commands =
  python -c 'print("ouch...")'

I removed the .tox directory, so tox has to recreate the virtualenv. I would expect that statsmodels dependencies are installed along with statsmodels. However, tox only installs statsmodels, no dependencies - that is no numpy, scipy, etc.
Can you reproduce that? I would expect to have statsmodels plus dependencies after the tox call.

@rgommers

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

That indeed fails, but it of course doesn't try to use the code in this PR. I don't see a good way of doing that with tox right now. tox just uses pip for installation, so if installing statsmodels in an empty virtualenv works, then tox will work for your project once this PR is in a released version of statsmodels.

@cel4

This comment has been minimized.

Copy link
Author

commented Aug 17, 2014

It actually should use the code of your pull request. I fetched your PR and uploaded it to a local pypi server using devpi.

@rgommers

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

ah OK, that would work. I've never used devpi before, but I'll give it a try.

@rgommers

This comment has been minimized.

Copy link
Member

commented Aug 17, 2014

Hmm, not much luck getting devpi to work quickly. I did check the tox source code, and see the following:

def hack_home_env(homedir, index_url=None):
    # XXX HACK (this could also live with tox itself, consider)
    # if tox uses pip on a package that requires setup_requires
    # the index url set with pip is usually not recognized
    # because it is setuptools executing very early.
    # We therefore run the tox command in an artifical home
    # directory and set .pydistutils.cfg and pip.conf files
    # accordingly.
    if not homedir.check():
        homedir.ensure(dir=1)
    d = dict(HOME=str(homedir))
    if not index_url:
        index_url = os.environ.get("TOX_INDEX_URL")
    if index_url:
        homedir.join(".pydistutils.cfg").write(
            "[easy_install]\n"
            "index_url = %s\n" % index_url)
        d["PIP_INDEX_URL"] = index_url
        d["TOX_INDEX_URL"] = index_url
    return d

So some hack is invoked here (numpy is in setup_requires now), and it uses easy_install. I'm afraid I don't really feel like trying to debug that.....

@cel4

This comment has been minimized.

Copy link
Author

commented Aug 18, 2014

To reproduce my tox run this should work:

  • pip install devpi
  • devpi quickstart
  • devpi upload from the statsmodels repository
  • tox -i http://localhost:3141/<user created in quickstart>/dev

To be honest, I'm not sure what this hack is for.
I see a general problem when I use pip to install statsmodels. setup_requires and install_requires are set, but pip simply ignores them.

@cel4

This comment has been minimized.

Copy link
Author

commented Aug 19, 2014

@rgommers, took way too long to debug this, but here are some details:
The filtering for egg_info seems to be the problem, see:
https://github.com/statsmodels/statsmodels/blob/master/setup.py#L385-L389
Using this code, pip installing into an empty environment will only install statsmodels, but not the required dependencies.
After removing 'egg_info' from the tuple in line 386, pip will also pull in all missing dependencies.

@jraby jraby referenced this issue Aug 21, 2014

Merged

Python extra req #3

@cel4

This comment has been minimized.

Copy link
Author

commented Aug 30, 2014

Any updates on this issue? Can you reproduce my problem when installing statsmodels in an empty virtualenv?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.