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

Add new format for add_colcounts(-, format = "N=xx (100%)") #516

Closed
alex-lauer opened this issue Feb 2, 2023 · 4 comments · Fixed by #521
Closed

Add new format for add_colcounts(-, format = "N=xx (100%)") #516

alex-lauer opened this issue Feb 2, 2023 · 4 comments · Fixed by #521

Comments

@alex-lauer
Copy link

Hi there,

I like rtables a lot and am always amazed by how lean the code for complicated tables can be using your package. One thing, my organization is overly focused on, is column counts being displayed as N=134 (100%). The display of 100%, to indicate the reference for all following percentages, can currently not be achieved with rtables.

Is this something you would consider addressing in a future update?

Thank a lot and best regards,
Alex

@gmbecker
Copy link
Collaborator

gmbecker commented Feb 3, 2023

This will be done by special casing formatting of the counts in the rendering machinery. A patch should do the following

  • check what dimension the column counts format is (match against formatters::list_valid_format_labels())
  • if its 1d, render as normal
  • if its 2d, check if its a percent format
    • if it is, append 1 (ie 100%) to the value and render
    • otherwise throw an error
  • if its 3d, throw an error

This type of special casing should not be done anywhere else, and is only going here because users don't have full control of the space of "Values" that counts can take, and there is no plan to give it to them.

@alex-lauer I am marking this as a good first issue to see if any of the team starting to work with me on aspects of rtables picks it up. If that doesn't happen after a reasonable amount of time, I'll go ahead and do it myself.

@edelarua edelarua self-assigned this Feb 5, 2023
@alex-lauer
Copy link
Author

Thanks @gmbecker, much appreciated!

@gmbecker
Copy link
Collaborator

gmbecker commented Feb 6, 2023

Reopening this as the functionality now works, but we need to add a format to formatters before we can give exactly the behavior asked for. (currently you'd get, e.g. 100 (100%) rather than N=100 (100%) with the above support. Will reclose when formattesr has been updated.

gmbecker added a commit to insightsengineering/formatters that referenced this issue Feb 6, 2023
gmbecker added a commit to insightsengineering/formatters that referenced this issue Feb 6, 2023
@gmbecker
Copy link
Collaborator

gmbecker commented Feb 6, 2023

Went ahead and made the change in formatters, so reclosing. Sorry for the noise everyone.

@gmbecker gmbecker closed this as completed Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants