implemented subsetting in recalculateResiduals #246
Conversation
|
Hi @EttnerAndreas - can perform a review for this PR, meaning that you look at the code if it makes sense, try out if everything works etc.? I should probably add a bit of documentation before we merge as well. |
|
Hi @florianhartig |
several checks passed, looks like function is still working, several checks for new selection function 'sel'. Any test implemented did pass the test. I would recommend merging and closing Issue #246
|
Hi Andreas, I don't understand the structure, I see you added some tests in this branch, but what is the purpose of this other branch https://github.com/florianhartig/DHARMa/tree/%23246 ? |
|
One comment about the tests: it's true that I don't really do this either very systematically, but ideal would be not only to run the function, but also to check if the outputs are correct. So e.g. you run various selections, or PIT vs. traditional residuals, but there are no checks to see if the function does what it is expected to do. Of course, currently it still checks if things run, which is better than nothing, but still ... |
|
OK, whatever, I will still merge now! |
|
In the beginning, I thought it might be good to use my own branch, but then I implemented the tests in your branch which is way more useful since there is the new code and you could undo it if Issues occur. |
|
OK, I understand. If you want to have an extra branch in this case, what you would usually do is to branch of from my development branch, then you can keep them updated, then merge back in the dev branch when finished. Or work in the dev branch directly |
related #245