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

ahr split function with simple = TRUE / FALSE #262

Closed
elong0527 opened this issue Jun 24, 2023 · 4 comments · Fixed by #270
Closed

ahr split function with simple = TRUE / FALSE #262

elong0527 opened this issue Jun 24, 2023 · 4 comments · Fixed by #270
Assignees
Labels
best-practice Enhance programming best practice

Comments

@elong0527
Copy link
Collaborator

elong0527 commented Jun 24, 2023

The output of a function should be the same structure.

The output wen simple = FALSE is not even generate AHR. Propose to split them into two functions:

ahr -> original one with simple = TRUE
ahr_hr -> report hr of each period.

>  ahr(enroll_rate = enroll_rate, fail_rate = fail_rate, total_duration = c(15, 30))
# A tibble: 2 × 5
   time   ahr event  info info0
  <dbl> <dbl> <dbl> <dbl> <dbl>
1    15 0.812  91.0  22.3  22.7
2    30 0.712 222.   54.4  55.4
> ahr(enroll_rate = enroll_rate, fail_rate = fail_rate, total_duration = c(15, 30), simple = FALSE)
# A tibble: 4 × 7
   time stratum     t    hr event  info info0
  <dbl> <chr>   <dbl> <dbl> <dbl> <dbl> <dbl>
1    15 All         0   1    53.9 13.5  13.5 
2    15 All         4   0.6  37.1  8.86  9.28
3    30 All         0   1    74.1 18.5  18.5 
4    30 All         4   0.6 147.  35.9  36.9 
@elong0527 elong0527 added the question Further information is requested label Jun 24, 2023
@elong0527 elong0527 added the best-practice Enhance programming best practice label Jun 24, 2023
@elong0527 elong0527 self-assigned this Jun 24, 2023
@nanxstats
Copy link
Collaborator

I completely agree. More generally, "type-stable" functions are preferred: https://design.tidyverse.org/out-type-stability.html

@LittleBeannie
Copy link
Collaborator

LittleBeannie commented Jul 14, 2023

Decision:
ahr -> original ahr function with simple = TRUE
pw_info -> original ahr function with simple = FALSE
ahr will call ``pw_info`

@LittleBeannie LittleBeannie removed the question Further information is requested label Jul 14, 2023
@elong0527
Copy link
Collaborator Author

Great! Maybe we can brainstorm a better function name.

If the function will become a general utility function, maybe piecewise_hazard_ratio

If the function only apply to AHR method, maybe starting from ahr_xxx e.g. ahr_piecewise.

@nanxstats, you must have better suggestion on name convention.

@LittleBeannie LittleBeannie linked a pull request Jul 14, 2023 that will close this issue
@LittleBeannie
Copy link
Collaborator

Great! Maybe we can brainstorm a better function name.

If the function will become a general utility function, maybe piecewise_hazard_ratio

If the function only apply to AHR method, maybe starting from ahr_xxx e.g. ahr_piecewise.

@nanxstats, you must have better suggestion on name convention.

Can we call it pw_info? Because it basically computes the statistical information for the piecewise model. Considering pw_info is also used in gs_info_wlr, let's not start it with ahr_xxx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
best-practice Enhance programming best practice
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants