Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests using testthat #11

Open
12 of 15 tasks
nikolase90 opened this issue Apr 4, 2019 · 2 comments
Open
12 of 15 tasks

Add tests using testthat #11

nikolase90 opened this issue Apr 4, 2019 · 2 comments
Assignees

Comments

@nikolase90
Copy link
Collaborator

nikolase90 commented Apr 4, 2019

If you're working on one of these files, please add the url to your branch or the pull request. Mark the box if the changes are merged with master.

R-files

src-files

The following files should not be tested

  • R/shapr-package.R
  • R/zzz.R
@nikolase90 nikolase90 created this issue from a note in Package development (To do) Apr 4, 2019
@nikolase90 nikolase90 self-assigned this Apr 4, 2019
@nikolase90 nikolase90 pinned this issue Jul 10, 2019
@nikolase90 nikolase90 moved this from To do to In progress in Package development Jul 10, 2019
Camiling added a commit that referenced this issue Aug 29, 2019
Plus adds extra checks to observation_impute
@martinju
Copy link
Member

I am getting a bit worried that every time we make a change to shap or explain functions, we have to re-create the test objects (explanation_explain_obj.rds and similar) which compares the results of the previous run to pass the tests. That means we don't have a good test for this. This is especially dangerous if one does many changes in the same PR (although we seldom do that).

My suggestion is to add a seperate test that checks only the shapley values from the different test models. We could run a lapply call to the ex_list, extract only the data.table with the shapley values, and then do a testthat::expect_known_value or testthat::expect_known_object on that. What do you think @nikolase90 ? Or is there a better option?

@nikolase90
Copy link
Collaborator Author

@martinju I think that sounds like a good way to do it.

@martinju martinju unpinned this issue Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Package development
  
In progress
Development

No branches or pull requests

2 participants