search

igrigorik / decisiontree

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

Improve Graph Output #30

Closed DannyBen closed 7 years ago

DannyBen commented 7 years ago

This PR comes to fix some obvious issues with the #graph method, in the least intrusive way possible,

Issue 1: Graph too small

The graph size too small for wide or tall trees - see #29 This is caused since they decided to set a size of "9,11" as the default for some reason, instead of letting dot do its thing naturally.

Fixed by providing an empty size to the DotGraphPrinter

Issue 2: Graphr dependency error

Since the graphr gem is not in the dependency list, a non friendly error was thrown when it was not found.

Fixed by rescuing LoadError and printing a friendly error message to STDERR

Issue 3: ID in node labels

The node labels included the object ID, which was necessary to distinguish between nodes, but not necessary for viewing.

Fixed by providing an alternative node_labeler proc to the DotGraphPrinter

Issue 4: Empty quotes in discrete labels

In discrete cases, the label included empty '' - for example, January ''. Not sure what it was there for, so I changed it (without changing how it behaves on continuous properties).

igrigorik commented 7 years ago

LGTM, and thank you!