Clean up requirements #1768
Clean up requirements #1768
Conversation
setup.py
Outdated
| @@ -83,7 +84,6 @@ | |||
| # add them to `docs/requirements.txt` | |||
| 'contextlib2', | |||
| 'graphviz>=0.8', | |||
| 'numpy>=1.7', | |||
There was a problem hiding this comment.
We could have removed this if all pytorch builds shipped with numpy. The pytorch pip wheel does not install numpy, but will throw an ImportError without numpy, because there are parts of torch that call numpy. I don't see any harm in keeping this in the requirement since it should anyways be installed with pytorch, and our minimum version requirement is an old one.
There was a problem hiding this comment.
Got it, thanks @neerajprad !
There was a problem hiding this comment.
It's probably worth adding a comment explaining why we need the numpy requirement, since a code comment is more discoverable than a github comment 😉
|
Why are the CodeCov comments back? |
| contextlib2 | ||
| cloudpickle>=0.3.1 |
There was a problem hiding this comment.
@jpchen can you confirm we no longer need cloudpickle or observations?
There was a problem hiding this comment.
yep LGTM. besides internal applications, i'm not sure which example was using cloudpickle in the first place...
| contextlib2 | ||
| cloudpickle>=0.3.1 |
There was a problem hiding this comment.
yep LGTM. besides internal applications, i'm not sure which example was using cloudpickle in the first place...
docs/requirements.txt
Outdated
| graphviz>=0.8 | ||
| observations>=0.1.4 |
There was a problem hiding this comment.
i think this is still used in the AIR example from #500
@neerajprad should we move off this dependency ?
There was a problem hiding this comment.
You are right, it seems that I miss that tutorial while using "grep".
There was a problem hiding this comment.
This is indeed needed by a few tutorials, let us reinstate it back.
This PR cleans up requirements in
setup.pyanddocs/requirement.txtfiles.@fritzo Is it fine to drop requirement for numpy now?