Vectorization work on clean_contents; Roxygen 6.1.0 build #80
Conversation
…ouble-loop and single loop. Removed commands from the loops that safely work in vector format.
…Rd files and NAMESPACE file).
|
It's removed half the NAMESPACE file. Not sure why. The roxygen in properties.R relies heavily on @evalnamespace and the make_namespace_S3_entries function. Check this still works. |
|
Yes sorry I noticed that and redid the fork; but then failed to push a fresh PR for it. Just pushed a fresh PR now.
Struggling to get the other components of that loop out - the double for loop should be replaceable with a mapply but haven’t quite got it sorted. The lower half looks like it should be a vector op. So the current PR still has the loops partially present - but even reducing what is in the loops is significant.
… On 16 Aug 2018, at 06:25, David Hugh-Jones ***@***.***> wrote:
It's removed half the NAMESPACE file. Not sure why. The roxygen in properties.R relies heavily on @evalnamespace and the make_namespace_S3_entries function. Check this still works.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#80 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADvnStdbQo1ckFsAcg1FyE-YmetA-4T-ks5uRQItgaJpZM4V-qYf>.
|
|
It might be helpful to run scripts/speed-test.R to see how your work
improves things. Feel free to add an as_Workbook test if there isn't one in
there. profvis is also helpful.
--
Sent from Gmail Mobile
|
|
Seems beneficial from the benchmark test we used above. Just annoyed I can’t seem to make mapply work to lose those loops entirely. Obvs the clean function is only part of it - but was the one with the more obvious solutions. Wonder for writing the sheets themselves if it is quicker to write the data once as strings and apply number formatting in a separate pass? |
|
Wonder for writing the sheets themselves if it is quicker to write the data
once as strings and apply number formatting in a separate pass?
I suspect so. Ideally keeping the end result unchanged from how it works
today (not because I'm convinced it's perfect, but just to avoid API
changes).
|
|
Decimal padding will take a bit more than this. The padding is done by adding spaces, the calculation has to be done on a per-column basis. (Not saying it can't be done in one run, just that the calculation varies for different columns.) |
Changes to try to remove as many calls from the nested loops in clean_contents as possible (the loops are still there but are smaller and should be removable).
Rebuild using Roxygen 6.1.0 (CRAN release) which has amended some build files slightly.