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 speed using stringi package #440

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

noamross
Copy link

This is an incomplete PR for feedback before I continue with it. It replaces a number of calls to paste0() and paste() with stringi::stri_join(), which is approximately 4x faster.

This came up in a project of mine where I needed to write a create a large number of styles and apply them to a large number of cells (in many workbooks!). We found a speed bottleneck in the paste() functions at https://github.com/awalker89/openxlsx/blob/master/R/WorkbookClass.R#L2105-L2106 , so these changes helped speed up the process and my team is using this fork.

If you are interested in this PR, I'd be happy to go through the rest of the package and replace calls (paste(), gsub(), grepl(), date formatting, etc.) that could be accelerated with their stringi equivalents, which in my experience are 2-4x faster.

stringi is a mixed bag as a dependency - it takes a while to install from source because it contains all of unicode, but it has no non-base dependencies and many people have it already as it is a tidyverse dependency. In my experience it is also quite stable.

@GegznaV
Copy link

GegznaV commented Oct 7, 2019

I think speed gives value. @noamross, Can this pull request be directed to the new home of openxlsx: https://github.com/ycphs/openxlsx/ (see ycphs/openxlsx#2)

@ycphs
Copy link
Contributor

ycphs commented Oct 8, 2019

Unfortunately the the checks fail. @noamross: Please could you check once more the. I found a problem in the saveWorkbook method.

The examples and the tests are showing the same failure.

ycphs added a commit to ycphs/openxlsx that referenced this pull request Nov 11, 2019
compare to Improve speed using stringi package #440 (awalker89/openxlsx#440)
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