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

Make numpy an optional dependency #212

Closed
wants to merge 1 commit into from
Closed

Conversation

pshchelo
Copy link

The code in websocket.py is able to work without numpy being installed,
but numpy is specified as a hard dependency in setup.py.

This change moves numpy to extras_require section.
To install websockify with fast HyBi support, one should install it with

pip install websockify[fastHyBi]

to have numpy also automatically installed.

to install with fast HyBi support, use smth like

`pip install websockify[fastHyBi]`
@jumpojoy
Copy link

I don't see any reason why we should install it permanently.

@DirectXMan12
Copy link
Member

We have already discussed this some in #135 and #163. The conclusion that we've come to in the past is that most people will probably want the significant performance gain that comes with using numpy. It is technically currently possible to use websockify without numpy, but it's not recommended.

Can you provide the use case that requires you to not have numpy?

@DirectXMan12 DirectXMan12 added feature New feature or request python labels Dec 1, 2015
@pshchelo
Copy link
Author

pshchelo commented Dec 2, 2015

websockify is a dependency for several OpenStack components (Nova, Ironic). OpenStack project has massive CI that deploys these components from git source with pip and installs dependencies. Currently AFAIK we do not actually have tests that would benefit from numpy acceleration in websockify, and definitely not one every gate job that deploys Nova (which is the basic core component of OpenStack). Installing numpy from source currently takes about 4 minutes, and if we could shave those off on almost every run, this would be great improvement.

Thus I thought that having a choice to install an actually optional component is reasonable.

@DirectXMan12
Copy link
Member

I'm hesitant to make numpy an optional dependency like that, since most users probably want the numpy acceleration (and I predict we'll get a lot of people who don't know why their new installs of websockify are suddenly slower).

Even though it's more hacky, I'd be more willing to implement a custom flag on the setup.py (which could be passed via a requirements.txt file as such: https://pip.pypa.io/en/latest/reference/pip_install/#per-requirement-overrides) that looked like --without-numpy-required or something like that (what is really needed is optional-with-default dependencies, but AFAIK there's no good way to do that).

I'm assuming that it's out of the question to just install a numpy from a distro package on the host system before the pip install command is run?

@kanaka, @samhed: what are your thoughts (I sympathize with @pshchelo WRT to the OpenStack build times ;-) )?

@samhed
Copy link
Member

samhed commented Jan 8, 2016

A flag for when you don't want numpy sounds good to me. I agree with @DirectXMan12 that the default behavior should be to use numpy.

@DirectXMan12
Copy link
Member

Unfortunately, it looks like due to the way pip installs packages, the option I suggested above will not work (requirements are collected first into the egg-info, requirements are installed, and then the main setup.py install is called).

I'll see if I can find a solution, but I'm tempted to just say the solution here is to install numpy from a distro package, and then pip should be able to use that version. Alternatively you could potentially place websockify in its own requirements.txt file, and call pip with --no-deps on that.

@CendioOssman
Copy link
Member

So close?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants