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

Develop format and rtf functions for exposure duration anaylsis #28

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

fukuhiro2023
Copy link
Collaborator

This PR includes:

  • Renaming format_base_char() and rtf_base_char() to format_sl_summary() and rtf_sl_summary() as the base function for each type of table.
  • Adding format_exp_duration() and rtf_exp_duration() for exposure duration analysis.
  • Defining format_base_char() and rtf_base_char() based on format_sl_summary() and rtf_sl_summary().

Copy link
Collaborator

@LittleBeannie LittleBeannie left a comment

Choose a reason for hiding this comment

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

There are lots of work! Thank you, @fukuhiro2023! I only have 2 very minor comments.

@@ -34,9 +34,9 @@
#' meta <- meta_sl_example()
#'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it a good idea if we add a comment explaining what the table mata |> prepare_sl_summary(...) |> formate_sl_summary(...) is for? I guess it is a baseline character table. If so, is it equivalent to the one from mata |> prepare_base_char(...) |> formate_base_char(...). If so, shall we say something here?

#'
#' @return RTF file and source dataset for baseline characteristic table.
#'
#' @export
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like the functions prepare_sl_summary() and format_sl_summary are 2 helper functions. I am wondering if we should un-export them...

Looping @wangben718 for a third opinion.

@wangben718 wangben718 merged commit d6e959b into main Jul 24, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants