search

igrigorik / decisiontree

ID3-based implementation of the ML Decision Tree algorithm
1.44k stars 130 forks source link

requiring graphr in production #11

Closed Numerico closed 11 years ago

Numerico commented 11 years ago

Hi, I'm using decisiontree for a rails app (thanks!). I found that my app was crushing at initialization time because of a failed require to graphr (http://pastebin.com/uMUDwfZf). I could work around it including graphr in my Gemfile... But I think that's wrong because graphr is a development dependency only. So I think the bug is rather to require graphr in id3_tree.rb, which will be executed for all environments. I removed it in my fork (https://github.com/Numerico/decisiontree/commit/6aa6e25cb6e941e14fb9c994ab06e4f851e8dd2b#L0L6) and two tests broke. I'd happily fix them and submit a pull request if you agree.

igrigorik commented 11 years ago

Shouldn't be an issue anymore. In practice, very few people use the actual graphing output, so making it a dependency seems like overkill.

Numerico commented 11 years ago

Thank you, I pulled it and all tests pass now. Wouldn't this desserve a new version in rubygems.org too?

igrigorik commented 11 years ago

Yep, I'll push out a new version.

lukeasrodgers commented 10 years ago

A new rubygems version would be great, as this issue has affected me as well.

igrigorik commented 10 years ago

</facepalm> ... just pushed out 0.5.0. Thanks for the reminder.

lukeasrodgers commented 10 years ago

Thanks!