-
Notifications
You must be signed in to change notification settings - Fork 49
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
Pagination and column widths handling #937
Conversation
i start thinking again that we need another downstream package for exporting |
The export functions are in {rtables} when available only for tables and {formatters} when for both tables and listings. Making a separate package would make sense if we wanted to cover more objects, but I do not see this happening soon. Also, exporters are relatively small; the larger function here is the flextable transformer, for which there are a lot of aesthetics that are hard to test. In summary, we already have a complex dependency tree, and we might want to keep dev exporters in {rtables} and full exporters (w/ listings capabilities) in {formatters}. Beside that I do not see too much the advantage of having exporters in a separate package atm. Also the risk of circular deps would be very real here, forcing us to move function around beyond what we need to. I am open to discussing this anyway! Maybe a pros and cons next PI? :) |
Unit Tests Summary 1 files 28 suites 1m 42s ⏱️ Results for commit c1960db. ♻️ This comment has been updated with latest results. |
Unit Test Performance DifferenceAdditional test case details
Results for commit 74b7a0c ♻️ This comment has been updated with latest results. |
…bles into 924_page_by_and_colwidths@main
Code Coverage Summary
Diff against main
Results for commit: c1960db Minimum allowed coverage is ♻️ This comment has been updated with latest results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to go!! It can be revisited at a later date if refinement is needed :)
Fixes #924 and hopefully #925
It seems that the pagination needs a lot of additional work to function correctly. It may be better to work on it in the next PI. @shajoezhu fyi here pagination works but column widths and lpp/cpp estimations are a bit difficult to concretize consistently