-
Notifications
You must be signed in to change notification settings - Fork 8
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 for ahr_blinded()
#435
Conversation
Focus on |
da289f1
to
7acc1c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @DMuriuki I rebased your PR so the merge conflicts are now resolved. Please take a look at this failing test and make necessary updates:
── Failure ('test-independent-ahr_blinded.R:17:3'): Piecewise model hr and intervals must be aligned ──
`ahr_blinded(hr = c(1, 3), intervals = c(3, 4))` did not throw an error.
You can use
git clone -b ahr-blinded-test https://github.com/Merck/gsDesign2.git
to clone the latest version of this branch to local, install, fix the test, and push.
a75b018
to
c4ef49a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LittleBeannie This PR is ready for review. 90% code coverage for ahr_blinded()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @DMuriuki and @nanxstats for the testing! Following, please find my comments.
@LittleBeannie Great, I've just removed the non-informative and unrealistic test cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot, @DMuriuki and @nanxstats !
Adding test file for the
ahr_blinded()
function.