pbiecek commented 6 years ago

Thanks @AleksandraDabrowska

Would you please:

add spaces before and after operators like < (ie x < 0 instead x<0, RStudio style guide)
the 1:(length(x[["position"]])) is risky (it vector is of the length 0), please use safer seq_along(x[["position"]])
in examples and tests it's better to have old examples and tests PLUS new examples and tests with new parameters, it's safer than replacing old examples and tests. In addition to plot(br, top_features = 2, min_delta = 0.01) it's good to have also the old plot(br) and plot(br, top_features = 2)

codecov-io commented 6 years ago

Codecov Report

Merging #20 into master will increase coverage by 0.63%. The diff coverage is 96.36%.

@@            Coverage Diff             @@
##           master      #20      +/-   ##
==========================================
+ Coverage   93.11%   93.75%   +0.63%     
==========================================
  Files           5        5              
  Lines         218      272      +54     
==========================================
+ Hits          203      255      +52     
- Misses         15       17       +2

Impacted Files	Coverage Δ
R/plot.R	`96.7% <96.36%> (-0.6%)`	:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d41f87c...b9f72dd. Read the comment docs.

AleksandraDabrowska commented 6 years ago

Thanks for your comments @pbiecek . I will add this changes in next commit.

pbiecek / breakDown

add top_features and min_delta to plot for broken #20

Codecov Report