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

should cvxopt be included in requirements.txt? #49

Closed
slivingston opened this issue Oct 2, 2017 · 6 comments
Closed

should cvxopt be included in requirements.txt? #49

slivingston opened this issue Oct 2, 2017 · 6 comments
Assignees
Labels
enhancement A new feature, an improvement, or other addition.
Milestone

Comments

@slivingston
Copy link
Member

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
Copy link
Member

johnyf commented Oct 2, 2017

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
Copy link
Member Author

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 johnyf self-assigned this Oct 5, 2017
@johnyf johnyf added enhancement A new feature, an improvement, or other addition. and removed discussion labels Oct 5, 2017
@johnyf johnyf added this to the 0.2.1 milestone Oct 5, 2017
@johnyf johnyf closed this as completed in 4a0146e Oct 5, 2017
@johnyf
Copy link
Member

johnyf commented Oct 9, 2017

Relevant to #15.

@slivingston
Copy link
Member Author

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
Copy link
Member

johnyf commented Oct 10, 2017

The suggested example worked on first try.

@slivingston
Copy link
Member Author

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 tulip-control/tulip-control#206.)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature, an improvement, or other addition.
Projects
None yet
Development

No branches or pull requests

2 participants