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.

Sign up for GitHub

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

Jump to bottom

rename time_unit_output to num_pt_year #973

Closed
Tracked by #978
clarkliming opened this issue Jun 19, 2023 · 3 comments · Fixed by #975
Closed
Tracked by #978

rename time_unit_output to num_pt_year #973

clarkliming opened this issue Jun 19, 2023 · 3 comments · Fixed by #975
Assignees
@edelarua
Labels
enhancement sme

Comments

@clarkliming
Copy link
Contributor

tern/R/control_incidence_rate.R

Line 26 in 395503f

time_unit_output = 1) {

the argument name is misleading. if time_unit_input is year, why time_unit_output become a number?
@edelarua edelarua self-assigned this Jun 19, 2023
@edelarua edelarua mentioned this issue Jun 19, 2023
Merged
@shajoezhu shajoezhu changed the title time_unit_input and time_unit_output rename time_unit_output to number_of_patient_year Jun 19, 2023
@shajoezhu
Copy link
Contributor

shajoezhu commented Jun 19, 2023 •
edited
Loading

rename time_unit_output to num_pt_year

@shajoezhu shajoezhu changed the title rename time_unit_output to number_of_patient_year rename time_unit_output to num_pt_year Jun 19, 2023
@clarkliming
Copy link
Contributor Author

please also consider the default the value: shall we use 1 or 100 ?

@edelarua
Copy link
Contributor

please also consider the default the value: shall we use 1 or 100 ?

@clarkliming I've updated the default value to 100. Thanks for the feedback!

@shajoezhu shajoezhu mentioned this issue Jun 20, 2023
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@edelarua edelarua

Labels
enhancement sme
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

More intuitive parameter names in control_incidence_rate insightsengineering/tern
3 participants
@shajoezhu @clarkliming @edelarua