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

PyTables 3.1.1 installation fails if numpy and numexpr are not preinstalled #413

Closed
dunpyl opened this issue Jan 7, 2015 · 14 comments
Closed

Comments

@dunpyl
Copy link

dunpyl commented Jan 7, 2015

PyTables 3.1.1 (as well as HEAD) attempts to import both numpy and numexpr, otherwise erroring out.

This behavior is a problem for anyone building deployment requirements in bulk, for example with pip install -r requirements.txt inside a virtualenv. The only way to handle it is install all other dependencies first, including numpy and numexpr, then install PyTables as a special extra step.

Instead, PyTables can require its depedencies in a way that would allow them all to be installed together from a single requirements file. For an example of this behavior implemented, see numexpr's setup.py.

@illume
Copy link

illume commented Jan 30, 2015

Yeah, it's broken with pip.

The way new pip works is different. It used to install them one at a time from the requirements.txt, but now does them in groups. Unfortunately this breaks the dependency checking for tables when using pip install -r requirements.txt as numpy and other deps aren't actually installed when it checks.

Removing the custom dependency checker and using the standard one would be better.

@FrancescAlted
Copy link
Member

I see, and I think this is a bad problem with new pip. I would be glad to patch this if you can send a pull request.

@andreabedini
Copy link
Contributor

Rewriting setup.py has been in the plans for a while. I'm on my iPad and I can't find it now but here should be a issue already open about this.

FrancescAlted added a commit that referenced this issue Apr 20, 2015
@FrancescAlted
Copy link
Member

Hey, I have tried to address concerns raised here in 9f77459. The general idea is that setuptools should be used if possible (before that had to be enforced by using the 'FORCE_SETUPTOOLS' environment variable), and hence everything should go more smoothly. Feedback on the commit above is appreciated.

Also, provided that we are distributing the beast with Cythonized module extensions, I am wondering why Cython is necessary for building. @avalentino any hints?

@avalentino
Copy link
Member

Also, provided that we are distributing the beast with Cythonized module extensions, I am wondering why Cython is necessary for building. @avalentino any hints?

Correct, cython is only needed for development or if one wants to build code checked out form the repo.

IMO it is safe to remove cython from (strict) dependencies even if we will loose the possibility to make something like the following:

$ pip install git+https://github.com/PyTables/PyTables.git@v.3.1.0#egg=tables

@illume
Copy link

illume commented Apr 20, 2015

Could it not check for the existence of a generated file, and add the
Cython dependency?

if not os.path.exists("ageneratedfile.c"):
dependencies.append("Cython")

cheerio,

On Mon, Apr 20, 2015 at 10:00 PM, Antonio Valentino <
notifications@github.com> wrote:

Also, provided that we are distributing the beast with Cythonized module
extensions, I am wondering why Cython is necessary for building.
@avalentino https://github.com/avalentino any hints?

Correct, cython is only needed for development or if one wants to build
code checked out form the repo.

IMO it is safe to remove cython from (strict) dependencies even if we will
loose the possibility to make something like the following:

$ pip install git+https://github.com/PyTables/PyTables.git@v.3.1.0#egg=tables


Reply to this email directly or view it on GitHub
#413 (comment).

@andreabedini
Copy link
Contributor

oh wow! great @FrancescAlted for tacking this!! I warn you There's a lot of outdated information out there about python packaging and the official reference is now available at https://packaging.python.org/en/latest/index.html and https://www.pypa.io/en/latest. Cython has also some info here http://docs.cython.org/src/reference/compilation.html#compilation-reference (but I wonder how up to date they are).

I agree we should check in the repository the cythonised extensions and drop the cython at runtime dependency.

I think setup.py is overly complicated and a lot of code can be removed.

@andreabedini
Copy link
Contributor

@FrancescAlted for reference I was experimenting with a drastically reduced version of setup.py (see https://github.com/andreabedini/PyTables/blob/setup/setup.py) but it wasn't working because of some compilation issues (see https://travis-ci.org/andreabedini/PyTables/builds/55252126). Possibly related to the 1.8.x HDF5 API forcing we are currently doing.

Happy to discuss (and experiment) more, hit me up on gitter

@FrancescAlted
Copy link
Member

Andrea, I understand your wish to make things as simple as possible, but what you are proposing is too radical IMO. But I certainly had a look at the docs you proposed and it seems to me that having a setup.py that requires setuptools is the way to go. So I did this trial: #438

Besides requiring setuptools (and hence, simplifying quite a few things), I have upgraded versions for HDF5 (1.8.7), numpy (1.7.1) and numexpr (2.4). Also, Cython is not a dependency anymore (except for generating .c sources, but this should be required for developers only). What people think?

@illume
Copy link

illume commented Apr 21, 2015

Hi,

this doesn't solve the pip problem.

You can't check the minimum requirements inside the setup file. Leave that
to pip.

+# Minumum equired versions for numpy, numexpr and HDF5 +exec(open
(os.path.join('tables', 'req_versions.py')).read())

On Tue, Apr 21, 2015 at 12:30 PM, FrancescAlted notifications@github.com
wrote:

Andrea, I understand your wish to make things as simple as possible, but
what you are proposing is too radical IMO. But I certainly had a look at
the docs you proposed and it seems to me that having a setup.py that
requires setuptools is the way to go. So I did this trial:

#438 #438

Besides requiring setuptools (and hence, simplifying quite a few things),
I have upgraded versions for HDF5 (1.8.7), numpy (1.7.1) and numexpr (2.4).
Also, Cython is not a dependency anymore (except for generating .c sources,
but this should be required for developers only). What people think?


Reply to this email directly or view it on GitHub
#413 (comment).

@FrancescAlted
Copy link
Member

I don't see the problem there because either min_numpy_version and min_numexpr_version are not being used inside setup.py anymore. Only min_hdf5_version is used, and this does not collide with pip.

@andreabedini
Copy link
Contributor

@FrancescAlted that's why I said I was experimenting :) #438 looks very good to me.

@andreabedini
Copy link
Contributor

I feel I need to apologise as I overlooked the import numpy problem. I try to solve this once for all in #439, @illume please test it out (using pip without pre-installing anything).

@andreabedini
Copy link
Contributor

This should be fixed by #438 and #439 included in the new 3.2 release. Feel free to reopen.

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

No branches or pull requests

5 participants