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

[FIX] Treeviewer sklearn tree compatibility #1870

Merged
merged 7 commits into from
Jan 12, 2017

Conversation

pavlin-policar
Copy link
Collaborator

@pavlin-policar pavlin-policar commented Jan 5, 2017

Issue

Trees output from Pythagorean forest are not compatible with the TreeViewer widget.

Description of changes

Having established that #1850 would cause far too much confusion and make the overall tree model code completely non-understandable (Adapter for sklearn trees, but now also vice versa, tree model for sklearn trees?), I've taken another stab at it.

It seems that converting the TreeViewer widget to use the TreeAdapter actually doesn't require any major changes at all. So this is what I did in this PR, TreeViewer now uses the TreeAdapter, just like PythagoreanTree, so at least it's less confusing.

This is still based on #1786 since that fixes the PythagoreanTree compatibility.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov-io
Copy link

codecov-io commented Jan 6, 2017

Current coverage is 89.26% (diff: 100%)

Merging #1870 into master will not change coverage

@@             master      #1870   diff @@
==========================================
  Files            86         86          
  Lines          9103       9103          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           8126       8126          
  Misses          977        977          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update d5e2882...19357a7

@pavlin-policar
Copy link
Collaborator Author

pavlin-policar commented Jan 11, 2017

@kernc Could you please take a look at this as well. This builds upon the PR closed yesterday that fixed compatibility between the Pythagorean forest and Pythagorean trees. This one changes TreeViewer to accept the input of Pythagorean forests as well.

This is kind of high priority and should be merged ASAP.

@astaric astaric changed the title Treeviewer sklearn tree compatibility [FIX] Treeviewer sklearn tree compatibility Jan 12, 2017
@astaric astaric merged commit aa9a09c into biolab:master Jan 12, 2017
@pavlin-policar pavlin-policar deleted the treeviewer-adapter branch January 12, 2017 11:16
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