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

Improve word doc formatting #212

Draft
wants to merge 54 commits into
base: develop
Choose a base branch
from
Draft

Conversation

kelliemac
Copy link
Contributor

@kelliemac kelliemac commented Jul 24, 2024

Description

Update word-styles-reference.docx for more professional-looking word document formatting, and switch to use of flextable package for table generation to ensure tables look good (and consistent) in both PDF and Word reports.

Related Issues

Related to #120

Checklist

  • This PR includes unit tests
  • This PR establishes a new function or updates parameters in an existing function
    • The roxygen skeleton for this function has been updated using devtools::document
  • I have updated NEWS.md to describe the proposed changes

kelliemac and others added 30 commits May 31, 2024 14:35
merge develop into this branch so up to date
@slager
Copy link
Contributor

slager commented Jul 25, 2024

"file15c072f288e5 study schema" in the table caption is expected I think, because that's the actual randomly generated name of the visc project folder created during the test knitting.

@kelliemac kelliemac marked this pull request as ready for review July 25, 2024 19:16
@kelliemac kelliemac requested review from slager and mayerbry July 25, 2024 19:16
@slager
Copy link
Contributor

slager commented Jul 25, 2024

I haven't looked at this carefully yet, but took a glance and one thing I think is to replace the word template header logos with the current VISC and SCHARP source images in the repo just to make sure they're the newest. The VISC logo, at least, looks like the larger, older one.

@mayerbry
Copy link
Contributor

imo, we need larger buy in from the analysis team before we switch between 'kableExtra and flextable. Currently, kable is a backbone of all of our reports and potentially part of our training materials. Switching the template might be abrupt without a bit of context and motivation.

This could include a short training (maybe a TIoT), examples/vignette of transitioning table code (I've noticed that there are different functions that do the same thing) so we avoid projects that have cross-dependencies/mixed table formats due to copy and pasting of working tables, and confirming that flextable can re-generate some of our recent tables that may not follow typical formatting (I can check a few of my PK ones).

VISCfunctions has functions that support input into kables, and so we should also identify what needs to be updated there.

@kelliemac
Copy link
Contributor Author

kelliemac commented Jul 25, 2024

imo, we need larger buy in from the analysis team before we switch between 'kableExtra and flextable. Currently, kable is a backbone of all of our reports and potentially part of our training materials. Switching the template might be abrupt without a bit of context and motivation.

This could include a short training (maybe a TIoT), examples/vignette of transitioning table code (I've noticed that there are different functions that do the same thing) so we avoid projects that have cross-dependencies/mixed table formats due to copy and pasting of working tables, and confirming that flextable can re-generate some of our recent tables that may not follow typical formatting (I can check a few of my PK ones).

VISCfunctions has functions that support input into kables, and so we should also identify what needs to be updated there.

@mayerbry yes I agree that this is a non-trivial change and we need to have some conversations and training before we merge it in! I already set up a TioT for 8/8 to talk about the advantages of moving to flextable and get feedback from the group on potential challenges that might come up. Can you post or link here an example or two of complicated kable formatting you've used in a previous project, so that I can make sure flextable can cover all of those needs? (I don't need to be able to run the code, just see what kable and kableExtra functionality is being used.)

I know VISCfunctions::pretty_pvalues() is one function that supports input into kables. Can you think of any other examples?

@kelliemac
Copy link
Contributor Author

I haven't looked at this carefully yet, but took a glance and one thing I think is to replace the word template header logos with the current VISC and SCHARP source images in the repo just to make sure they're the newest. The VISC logo, at least, looks like the larger, older one.

thanks @slager. I forgot that you had changed the VISC logo too, so that was the only one I didn't change. just fixed it.

@slager
Copy link
Contributor

slager commented Jul 25, 2024

I agree with this need to think hard about it. This is amazing for these tables, yet I still find myself thinking about my work on QC reports, where I've seen there's often a need to customize a table pretty extensively to meet various stakeholder needs. I've already seen a lot of edge cases with tables just in the several QC reports I've done, like whether it's extra wide, how many levels of columns we're collapsing rows on, whether the table occupies more than 1 page, whether we want certain groups of rows to stay together when they get split across pages, if there are non-ASCII characters in labels, adding another caption, etc. That has often required custom workarounds to get the right data visualization.

It's hard to test for all the edge cases for a function which has many potentially interacting arguments and whose output is a side effect that needs to be evaluated visually. With kableExtra, there's documentation to hopefully find a solution, but if we make our own big function, it will be on us, and potentially harder to customize when we need to. So definitely some pros and cons.

Looking forward to the TIOT!

@kelliemac
Copy link
Contributor Author

Just to clarify in response to one of your comments @slager: we are no longer creating a custom VISCtemplates table-generating function. We are just encouraging people to use the flextable package instead of kable and kableExtra, so they should still be able to look up the flextable documentation for additional capabilities or issues, rather than turning to us as the first line of defense.

@kelliemac
Copy link
Contributor Author

In my experience, flextable seems to have all of the key capabilities that are present in kableExtra. I'm sure there are some cases where they don't perfectly line up, but for the most part they just use different syntax to achieve the same kinds of effects. I can start drafting a list of kableExtra formatting calls and their equivalent flextable calls.

here is the flextable website for reference: https://davidgohel.github.io/flextable/

@slager
Copy link
Contributor

slager commented Jul 25, 2024

Oops, I hadn't noticed yet that it is no longer a custom function, so ignore the comments of mine related to that!

@mayerbry
Copy link
Contributor

Looking forward to that TIOT, hopefully we get some good discussion!

To clarify too, I don't plan to champion kable and kableExtra, which have their own annoying quirks. I would love to copy and paste a nicely formatted table straight out of word report into manuscript! I just want to make sure that flextable has some of the features we typically use.

That report came to mind quickly for me, but I'll need to think about other examples. I can post them here when I stumble across them

@kelliemac
Copy link
Contributor Author

thanks @mayerbry! I will check on those elements sometime next week to see how flextable handles them. and yeah just post more thoughts here about important functionality as you think of it and I'll add that to the queue!

sidenote: I think it will be good anyways to have these examples in the skeleton.Rmd file's example table so that that example is more comprehensive.

@slager
Copy link
Contributor

slager commented Jul 31, 2024

flextable depends on textshaping, which is tricky to install on statsrv. To install flextable, first install textshaping following these instructions, and then install flextable. This method worked great for me:

=-=-=

Hi Dave,

I hope all is well. I was able to test installing the R package 'textshaping'.

To get the package to install on your local R environment you will have to do a few things to get it working.

1.) export the PKG_CPPFLAGS_SITE (export PKG_CPPFLAGS_SITE="-I/usr/include/fribidi -I/usr/include/harfbuzz")

2.) Once you are within your R shell you will want to run: options(repos='https://p3m.dev/cran/2021-02-15'); You could try a more recent date but we can't assure that the newer version will work in our environment. (Dave note: I didn't do this because the most recent version works fine)

3.) run the install.package('textshaping')command

You will want to make sure to be running all of these commands from admin-os.pc.scharp.org. Doing this will also make sure that the package is installed on statsrv/cronsrv/rstudio-server.

If you run into any issues or have any additional questions just reach out to me.

Thanks,

Deon

@kelliemac kelliemac marked this pull request as draft August 23, 2024 22:46
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.

3 participants