tulip-control / polytope

Geometric operations on polytopes of any dimension
https://pypi.org/project/polytope
Other
73 stars 19 forks source link

should `cvxopt` be included in requirements.txt? #49

Closed slivingston closed 6 years ago

slivingston commented 6 years ago

On the current tip of master branch, cvxopt is included in requirements.txt. There is a comment above it to indicate that it is optional, but it has no semantic impact on the file.

I propose that it is removed. Thoughts?

My main motivation is to have requirements.txt only enforce required dependencies to improve the reliability of using pip install -r requirements.txt. Installing cvxopt is still somewhat difficult to reliably automatically install from source code.

According to the documentation of pip install, it is possible to use -r option multiple times, i.e., to give multiple requirements file. One idea is to create a separate extra_requirements.txt that include optional dependencies like cvxopt. Then, a full install can be achieved with

pip install -r requirements.txt -r extra_requirements.txt

while automated enviroments like Binder will continue to use the standard pip install -r requirements.txt.

johnyf commented 6 years ago

I agree with omitting optional dependencies as for example cvxopt from requirements.txt. One possibility is to comment them, letting the user uncomment them as needed. Optional dependencies that are easy to install (e.g., pure Python and available from PyPI), if any (why omit them from install_requires if they pose no challenges), could remain in requirements.txt.

One benefit of commenting those dependencies is that an unfamiliar user would see them listed among the other entries. On the other hand, designing a developer's file for an unfamiliar user is an oxymoron.

I think that establishing semantic separation is a solid argument. I would like to avoid increasing the number of files in the repository's root, but otherwise I am fine with the alternatives. That install_requires includes required dependencies suggests putting only extras in requirements.txt, but on the other hand install_requires does not pin the dependencies, which is what requirements files are for.

Looking at what the widely used Python packages do, I liked the most the approach of networkx: a directory named requirements. I suggest that we use this approach, which accommodates for multiple requirements files:

  1. without producing any clutter in the top directory, and
  2. while keeping it easy to find where the requirements files are located (instead of putting them under a directory with a different name).

This approach makes it possible to also include a README under the requirements/ directory, as networkx demonstrates. Another little detail is that in such a directory, the prefix "requirements" can be dropped, so the files become requirements/default.txt, requirements/extras.txt etc., which requires about the same amount of typing (this is a secondary observation of course).

Unlike networkx though, I think that requirements files should pin dependencies, in accord with standard practice for ensuring reproducibility.

slivingston commented 6 years ago

I agree with your proposal to follow the practice of the networkx project except that we pin dependency versions, i.e., use == instead of >= for requirement specifiers.

Note that networkx also uses requirements.txt in the root of the sourcetree. It only includes paths to the actual requirements files. It might be useful for us to do this, too, because it ensures that automation tools use the correct requirements.txt file.

johnyf commented 6 years ago

Relevant to #15.

slivingston commented 6 years ago

Now Binder can be used. To generate random polytopes in your web browser, go to https://beta.mybinder.org/v2/gh/tulip-control/polytope.git/master, launch, and try

%matplotlib inline
import numpy as np
import polytope as pc

V = np.random.random((5, 2))
print(V)

P = pc.qhull(V)
print(P)

P.plot()
johnyf commented 6 years ago

The suggested example worked on first try.

slivingston commented 5 years ago

The example above can also be run on Colab with GLPK after the following:

!curl -L -O https://github.com/tulip-control/polytope/archive/master.zip
!unzip master.zip
!apt-get install libglpk-dev
!export CVXOPT_BUILD_GLPK=1 && cd polytope-master && pip install -r requirements/extras.txt
!cd polytope-master && pip install .

I recommend that we provide several examples in Jupyter notebooks that can be run on Binder and Colab. (This is related to https://github.com/tulip-control/tulip-control/issues/206.)

temporary demo: https://colab.research.google.com/drive/1rLdDAke83DgzkciJUVGiAJX4BSJeUNsI