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

adds ExpHazOS function #65

Merged
merged 7 commits into from
Sep 20, 2023
Merged

adds ExpHazOS function #65

merged 7 commits into from
Sep 20, 2023

Conversation

holgstr
Copy link
Contributor

@holgstr holgstr commented Sep 19, 2023

Pull Request

adds ExpHazOS function

@holgstr holgstr added the enhancement New feature or request label Sep 19, 2023
@holgstr holgstr self-assigned this Sep 19, 2023
Copy link
Collaborator

@danielinteractive danielinteractive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @holgstr ! Nice first PR - please see few comments below. In addition please add this function to the pkgdown yaml file

R/ExpHazOS.R Outdated Show resolved Hide resolved
R/ExpHazOS.R Outdated Show resolved Hide resolved
R/ExpHazOS.R Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Sep 19, 2023

badge

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  ---------
R/assertions.R                  15       0  100.00%
R/eventTracking.R               88       2  97.73%   48, 244
R/getClinicalTrials.R           64       0  100.00%
R/getSimulatedData.R           107       0  100.00%
R/getWaitTimeSum.R              29       2  93.10%   45, 48
R/hazardFunctions.R              6       0  100.00%
R/piecewiseDistribution.R       32       0  100.00%
R/piecewiseHazards.R            18       0  100.00%
R/survivalFunctions.R           96       0  100.00%
R/transitionParameters.R        42       0  100.00%
TOTAL                          497       4  99.20%

Diff against main

Filename                     Stmts    Miss  Cover
-------------------------  -------  ------  --------
R/eventTracking.R               +8       0  +0.23%
R/getClinicalTrials.R           +6       0  +100.00%
R/getSimulatedData.R            +5       0  +100.00%
R/hazardFunctions.R             +6       0  +100.00%
R/piecewiseDistribution.R       -1       0  +100.00%
R/piecewiseHazards.R            +1       0  +100.00%
R/survivalFunctions.R          +96       0  +100.00%
TOTAL                         +121       0  +0.26%

Results for commit: e84dcd8

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@danielinteractive
Copy link
Collaborator

Oh and we need unit tests

@holgstr
Copy link
Contributor Author

holgstr commented Sep 19, 2023

Hey @danielinteractive, should have included all of your feedback except the comment regarding the pkgdown yaml. Is there an automatic function call to do this efficiently just for one function added to the package?

@github-actions
Copy link
Contributor

github-actions bot commented Sep 19, 2023

Unit Tests Summary

    1 files    13 suites   2m 28s ⏱️
  52 tests   52 ✔️ 0 💤 0
129 runs  129 ✔️ 0 💤 0

Results for commit 20c1ffb.

♻️ This comment has been updated with latest results.

@danielinteractive
Copy link
Collaborator

You can just edit the https://github.com/insightsengineering/simIDM/blob/main/_pkgdown.yaml file manually as part of this PR. There is no more automatic way to do that (AFAIK)

@holgstr
Copy link
Contributor Author

holgstr commented Sep 20, 2023

_pkgdown.yaml is updated now

@danielinteractive
Copy link
Collaborator

cool, now you just need to fix the unit test to avoid 0 hazard values, see the R CMD check log (locally, or hidden behind the red cross for the commit where it failed):
image

@danielinteractive
Copy link
Collaborator

so now as soon as all checks are green (passed) then you can hit the Squash and Merge button

@holgstr
Copy link
Contributor Author

holgstr commented Sep 20, 2023

@danielinteractive cool, thanks! :)

@holgstr holgstr merged commit dd93072 into main Sep 20, 2023
25 checks passed
@holgstr holgstr deleted the exp-haz-os branch September 20, 2023 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants