search

ModelOriented / iBreakDown

Break Down with interactions for local explanations (SHAP, BreakDown, iBreakDown)
https://ModelOriented.github.io/iBreakDown/
GNU General Public License v3.0
82 stars 15 forks source link

DALEXverse 0.19.8 release summer 2019 #57

Closed pbiecek closed 5 years ago

pbiecek commented 5 years ago

DALEXverse 0.19.8 release summer 2019

Integration

assigned: @pbiecek

Code review

assigned: @maksymiuks

Feature review

assigned: @kasiapekala

maksymiuks commented 5 years ago
hbaniecki commented 5 years ago

remove caret from suggest

hbaniecki commented 5 years ago

change URL in DESCRIPTION to github + add demo.gif

kasiapekala commented 5 years ago

tests https://github.com/ModelOriented/iBreakDown/blob/master/tests/testthat/test_break_down.R adding break_down with interactions here, along the bd_rf, and similarly testing for the break_down class, would increase the code coverage

break_down, local_interactions, local_attributions https://github.com/ModelOriented/iBreakDown/blob/master/R/local_attributions.R#L5 https://github.com/ModelOriented/iBreakDown/blob/master/R/local_interactions.R#L6 I was going to suggest using \code{\link[breakDown]{break_down}}, but couldn't find function break_down() in package breakDown.

https://github.com/ModelOriented/iBreakDown/blob/master/R/local_interactions.R#L14 could be changed to: "By default it is equal to '1'. The larger the number is, the more frequently interactions will be presented in explanations."

break_down_uncertainty https://github.com/ModelOriented/iBreakDown/blob/master/R/break_down_uncertainty.R# calles -> calls calculated -> calculates

https://github.com/ModelOriented/iBreakDown/blob/master/R/break_down_uncertainty.R#L6 without "by"

describe https://github.com/ModelOriented/iBreakDown/blob/master/R/describe_breakdown.R#L12 variables counteracting with each other -> variables counteracting each other

https://github.com/ModelOriented/iBreakDown/blob/master/R/describe_breakdown.R#L14 choosen -> chosen defalut -> default dicribes -> describes which chooses three most significant variables -> maybe would be better to avoid repeating the verb and write "which selects three most significant variables"

https://github.com/ModelOriented/iBreakDown/blob/master/R/describe_breakdown.R#L19 from the description of the short_description parameter and from this line is not obvious for me whether the short description is returned instead of the longer one or somehow along it

https://github.com/ModelOriented/iBreakDown/blob/master/R/describe_breakdown.R#L14 https://github.com/ModelOriented/iBreakDown/blob/master/R/describe_breakdown.R#L22 argumentation_mode is mentioned twice (in slightly contradictory ways), but I don't see it in describe() parameters

https://github.com/ModelOriented/iBreakDown/blob/master/R/describe_breakdown.R#L3 description -> descriptions what enchaces -> that enchace

plots and prints https://github.com/ModelOriented/iBreakDown/blob/master/R/plot_break_down_uncertainty.R description could elaborate more on what exactly is plotted, since the break_down_uncertainty object is a little bit more complex than break_down object

https://github.com/ModelOriented/iBreakDown/blob/master/R/plot_break_down.R#L4 missing \code links

https://github.com/ModelOriented/iBreakDown/blob/master/R/plot_break_down.R#L6 https://github.com/ModelOriented/iBreakDown/blob/master/R/plotD3_shap.R#L6 https://github.com/ModelOriented/iBreakDown/blob/master/R/plot_break_down_uncertainty.R#L3 repeated word 'model' (the same for print.break_down)

https://github.com/ModelOriented/iBreakDown/blob/master/R/plot_break_down.R#L9 (the same for plotD3_break_down.R and plotD3_shap.R) By deafult NA therefore will be extracted from -> By default NA, therefore it will be extracted from But can be set to some constants, usefull if these plots are used for comparisons. -> But it can be set to some constants, useful if these plots are to be used for comparisons.

https://github.com/ModelOriented/iBreakDown/blob/master/R/plot_break_down.R#L10 if TRUE, variable contributions will be added to the plot

https://github.com/ModelOriented/iBreakDown/blob/master/R/plot_break_down.R#L15 (the same for plotD3_break_down.R, plotD3_shap.R, print.break_down) function that is to used for -> function to be used for Second sentence could be changed for: This should be \code{\link{signif}} which keeps a specified number of significant digits or \code{\link{round}} (which is default) to have the same precision for all components.

https://github.com/ModelOriented/iBreakDown/blob/master/R/plot_break_down.R#L11 how much labels sholud be shifted right as a fraction of range. By default 0.05 -> number describing how much labels should be shifted to the right, as a fraction of range. By default equal to 0.05.

https://github.com/ModelOriented/iBreakDown/blob/master/R/plot_break_down_uncertainty.R#L5 I think words 'logical' and 'default' are not necessary

https://github.com/ModelOriented/iBreakDown/blob/master/R/plotD3_break_down.R#L5 missing \code links

https://github.com/ModelOriented/iBreakDown/blob/master/R/plotD3_break_down.R#L18 (the same for plotD3_shap.R) if TRUE, the height of plot scales with window size

https://github.com/ModelOriented/iBreakDown/blob/master/R/plotD3_break_down.R#L21 (the same for plotD3_shap.R) By default NA therfore will choose DrWhy colors -> If NA (default), DrWhy colors are used.

https://github.com/ModelOriented/iBreakDown/blob/master/R/plotD3_shap.R#L4 \code{\link{shap}}

AdamIzdebski commented 5 years ago

remove caret from suggest

It is needed for building vignettes