Improved label printing/plotting of nodes/leaves

[roland-KA]

• printnode is now rounding the thresholds to sigdigits significant digits (sigdigits is a keyword parameter)
• the label "Class: " is only printed/plotted within leaves, if ids are used as 'class names'
• a clarification and a check has been added, that you cannot add class names with wrap if there are already class names in use (i.e. existing class names cannot be overriden, they can only be added if ids are used within the original decision tree structure).

[codecov-commenter]

Codecov Report

Merging #200 (d7fccfa) into dev (a072539) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #200      +/-   ##
==========================================
+ Coverage   87.97%   87.99%   +0.01%     
==========================================
  Files          10       10              
  Lines        1247     1249       +2     
==========================================
+ Hits         1097     1099       +2     
  Misses        150      150              

Impacted Files	Coverage Δ
src/abstract_trees.jl	91.30% <100.00%> (+0.82%)	⬆️

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

[rikhuijzer]

Thanks for working on this.

With this PR, the sigdigits keyword argument can not be modified by the user, correct? It is called from AbstractTrees.print_tree, but the default interface for AbstractTrees.printnode has no sigdigits keyword argument, so that will not be used.

Should we perhaps call AbstractTrees.print_tree(f::Function, io::IO, tree; kwargs...) where f is a custom implementation of printnode with signature f(io::IO, node) to fix this? I don't see yet how, but maybe you or Anthony does. I'm having a hard time with all the print logic and figuring out what is called when.

In any way, I would say this PR is good to go with the default keyword argument so that the issue with TreeRecipe.jl can be fixed (#197). Probably it would be best to make it a breaking release to ensure that we're not accidentally breaking someone's code.

[roland-KA]

With this PR, the sigdigits keyword argument can not be modified by the user, correct? It is called from AbstractTrees.print_tree, but the default interface for AbstractTrees.printnode has no sigdigits keyword argument, so that will not be used.

That's correct.

Should we perhaps call AbstractTrees.print_tree(f::Function, io::IO, tree; kwargs...) where f is a custom implementation of printnode with signature f(io::IO, node) to fix this? I don't see yet how, but maybe you or Anthony does. I'm having a hard time with all the print logic and figuring out what is called when.

I've also had some thoughts on how to make sigdigits modifiable by the user. To make it work with print_tree, your suggestion is the right way to go. But our actual goal is to use it in the plot recipe (where I use printnode directly). And there I have to do some rework on several functions.

In any way, I would say this PR is good to go with the default keyword argument so that the issue with TreeRecipe.jl can be fixed (#197). Probably it would be best to make it a breaking release to ensure that we're not accidentally breaking someone's code.

Because of the rework needed in TreeRecipe.jl, I agree with you, that we should merge this PR as is and postpone the rest to another update.

As TreeRecipe.jl isn't a registered package yet, I don't think that there are any users out there whose code we could brake. But if you feel safer, just make it a breaking release.

And thank's for the improvements you suggested! 👍

[ablaom]

Thanks @roland-KA for chipping in here again.

Unless the new check is ruling out previously allowed behaviour (see my comment above) - which I hope we can avoid, I think this can be tagged non-breaking.

[ablaom]

This is now good to go.

Thanks again @roland-KA and @rikhuijzer