Adding feature_importances to report for DecisionTree models

[john-waczak]

This update adds feature_importances to the report for each model from DecisionTrees.jl using the impurity_importance function. I've followed the same output format as the feature_importances from EvoTrees.jl and added a simple test for each method to runtests.jl.

[ablaom]

@OkonSamuel

[OkonSamuel]

This update adds feature_importances to the report for each model from DecisionTrees.jl using the impurity_importance function. I've followed the same output format as the feature_importances from EvoTrees.jl and added a simple test for each method to runtests.jl.

I can see here that you are using impurity_importance. I suggest adding a new parameter to DecisionTree models that would allow the user to specify the method to use in computing feature importance.

[john-waczak]

Are you suggesting I add a new hyperparamter to the structs in MLJDecisionTreeInterface.jl? If not, where should this go?

[OkonSamuel]

Are you suggesting I add a new hyperparamter to the structs in MLJDecisionTreeInterface.jl? If not, where should this go?

Yes.

[john-waczak]

@OkonSamuel

Okay, now we've got the ability to choose from :none, :impurity, and :split. I wasn't sure how to deal with the permutation_importance function generically as it has a different signature than the other two. Any suggestions?

[OkonSamuel]

@OkonSamuel

Okay, now we've got the ability to choose from :none, :impurity, and :split. I wasn't sure how to deal with the permutation_importance function generically as it has a different signature than the other two. Any suggestions?

Don't worry about permutation importance for now. That would added later to MLJBase.

[john-waczak]

Okay, sounds good. Is there anything else that needs to be done?

[codecov-commenter]

Codecov Report

Merging #28 (0b3b19f) into dev (d94341a) will increase coverage by 0.67%.
The diff coverage is 89.74%.

@@            Coverage Diff             @@
##              dev      #28      +/-   ##
==========================================
+ Coverage   86.20%   86.88%   +0.67%     
==========================================
  Files           1        1              
  Lines          87      122      +35     
==========================================
+ Hits           75      106      +31     
- Misses         12       16       +4     

Impacted Files	Coverage Δ
src/MLJDecisionTreeInterface.jl	86.88% <89.74%> (+0.67%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[OkonSamuel]

@john-waczak. I have reviewed your PR and suggested changes.

[john-waczak]

@OkonSamuel Okay, I think I have it updated to follow the format now. Apologies for the confusion; I was using EvoTrees.jl as an example and thought they needed to be in the report. I ended up adding features to the report of each model so they'd be available for the feature_importances function. I guess EvoTrees.jl will need to be updated next, right?

[OkonSamuel]

@OkonSamuel Okay, I think I have it updated to follow the format now. Apologies for the confusion; I was using EvoTrees.jl as an example and thought they needed to be in the report. I ended up adding features to the report of each model so they'd be available for the feature_importances function. I guess EvoTrees.jl will need to be updated next, right?

Yes. We plan to make changes to several packages including EvoTrees.jl

[OkonSamuel]

Thanks @john-waczak for all your effort. Just a few things that needs to be addressed and this PR would be ready for merge.

[john-waczak]

@OkonSamuel Glad to help!

[ablaom]

Okay, great to have this. I'll tag a new release.