Skip to content

Remove scikit-learn dependency#2192

Merged
shelhamer merged 2 commits intoBVLC:masterfrom
lukeyeager:remove-scikit-learn
Mar 27, 2015
Merged

Remove scikit-learn dependency#2192
shelhamer merged 2 commits intoBVLC:masterfrom
lukeyeager:remove-scikit-learn

Conversation

@lukeyeager
Copy link
Copy Markdown
Contributor

As far as I can tell, scikit-learn is never used. I get no results when I search for either "scikit-learn" or "sklearn." If it is really not used, can we remove it from requirements.txt? This is the only pip dependency which doesn't have a debian installer in Ubuntu 14.04 (besides Pillow>=2.7.0, which I'll address separately).

FWIW, all tests still pass without scikit-learn installed, but I don't trust the python test coverage much.

@shelhamer
Copy link
Copy Markdown
Member

sklearn is used in our vector data / HDF5 example for comparison. The dependency could be noted in that example instead I suppose, since Caffe itself never makes use of the module as you noted.

@lukeyeager
Copy link
Copy Markdown
Contributor Author

sklearn is used in our vector data / HDF5 example for comparison

Ah, I didn't see that. Strange that it didn't show up in a repository-wide search.

@lukeyeager
Copy link
Copy Markdown
Contributor Author

I added a comment in the example, is that sufficient?

@shelhamer
Copy link
Copy Markdown
Member

Yeah, this seems fine. @longjon how do you feel about not forcing dependencies needed in an example?

@longjon
Copy link
Copy Markdown
Contributor

longjon commented Mar 27, 2015

@shelhamer I'm fine with not forcing dependencies for examples if clearly noted.

shelhamer added a commit that referenced this pull request Mar 27, 2015
Remove scikit-learn dependency -- the need is noted in the relevant example
@shelhamer shelhamer merged commit 12f559e into BVLC:master Mar 27, 2015
@lukeyeager lukeyeager deleted the remove-scikit-learn branch March 27, 2015 23:10
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

Successfully merging this pull request may close these issues.

3 participants