Code review for v 1.0.0

[pbiecek]

What to check:

• is documentation sufficient to understand function's parameters
• is the description in DESCRIPTION up to date and meaningful
• are examples easy to understand and follow
• are descriptions in vignettes easy to understand and follow
• is the README.md file easy to understand, consistent with other descriptions
• is the R code easy to understand and follow
• R code should be modular (long functions are bad)
• are function names and variable names consistent

[agosiewska]

Comments for Thursday

• Link to D3 example in README doesn't work

• it would be good to change names of files in R folder, they should be consistent wit names of functions inside them

• unmeaningful name of the variable - tmp. How about path or something similar? I also think that this code should be a separate function, e.g. make_path() or create_path() https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/break_down.R#L113-L127 The same for https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/break_interactions.R#L157-L169

- how about moving parameter interaction_preference after label? The order of parameters common with function local_attributions would overlap. https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/break_interactions.R#L88-L93 https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/break_down.R#L85-L89

• codes that are repeated in local_attributions and local_interactions. It would be good to make code DRY. https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/break_down.R#L90-L98 https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/break_interactions.R#L94-L101

• unmeaningful variable name - tmp, maybe differences_1d or differences_summary? https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/break_interactions.R#L128-L135 The same for: https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/break_interactions.R#L149-L155

• these codes differ a little, would it be possible to make them identical and make a separate function? https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/break_down.R#L168-L182 https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/break_interactions.R#L206-L220

[agosiewska]

• How about adding a second class data.frame for break_down objects?

- names of parameters in plotD3() and plot() are inconsistent https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/plotD3.R#L55-L59 https://github.com/ModelOriented/iBreakDown/blob/1096fa4482dc0d8fb6b4ef202b0a87277246e4a2/R/plot.R#L83-L85

• I think that data manipulations in plotD3() and plot() should be separate functions.

• now we can use a ggplot2 theme from DALEX, This file is unnecessary: https://github.com/ModelOriented/iBreakDown/blob/master/R/theme_drwhy.R

[pbiecek]

Most of these issues get fixed. Thanks. Code is refactored and simplified, but the part for interactions is not a production quality code.

Once the methodology behind interactions will be established we need to rewrite local_interactions