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

Support pagination of lists of rtables/rlistings objects #213

Merged
merged 35 commits into from
Apr 3, 2024

Conversation

edelarua
Copy link
Contributor

Closes #212

@edelarua edelarua added the sme Tracks changes for the sme board label Oct 14, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 14, 2023

badge

Code Coverage Summary

Filename             Stmts    Miss  Cover    Missing
-----------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------------------------------------------
R/format_value.R       194      12  93.81%   88, 104-111, 189, 301, 401, 412, 420
R/generics.R           108       8  92.59%   454, 466, 499, 528, 643, 663-669
R/labels.R              55       7  87.27%   49, 55, 64, 105, 133, 142, 146
R/matrix_form.R        560      38  93.21%   103, 459-460, 548, 561-564, 583, 615, 709-710, 725-730, 760-763, 796-797, 829-830, 874, 1050, 1086, 1089-1093, 1140, 1191, 1194, 1198
R/mpf_exporters.R      257      17  93.39%   2, 100-102, 180, 220, 225, 409-415, 419, 422, 460, 556
R/page_size.R           45       1  97.78%   172
R/pagination.R         670      38  94.33%   302-305, 407-420, 506, 582, 765-766, 787-797, 1011-1012, 1195, 1202, 1281, 1416-1417, 1429-1430, 1444-1445
R/tostring.R           586      49  91.64%   41, 93, 162, 196, 204, 240, 297-300, 393-397, 400-403, 410-415, 492, 631-632, 697-704, 761-765, 849, 864, 958, 1010, 1051, 1096, 1151, 1158
R/utils.R                3       0  100.00%
R/zzz.R                 17       6  64.71%   29-34
TOTAL                 2495     176  92.95%

Diff against main

Filename             Stmts    Miss  Cover
-----------------  -------  ------  -------
R/generics.R            +3      +1  -0.74%
R/matrix_form.R         +8     -12  +2.27%
R/mpf_exporters.R      +19      -2  +1.37%
R/pagination.R         +36     -11  +2.06%
TOTAL                  +66     -24  +1.18%

Results for commit: 25396b0

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Oct 14, 2023

Unit Tests Summary

  1 files    5 suites   13s ⏱️
 46 tests  46 ✅ 0 💤 0 ❌
322 runs  322 ✅ 0 💤 0 ❌

Results for commit 25396b0.

♻️ This comment has been updated with latest results.

@shajoezhu shajoezhu marked this pull request as draft October 14, 2023 06:18
R/mpf_exporters.R Outdated Show resolved Hide resolved
R/mpf_exporters.R Outdated Show resolved Hide resolved
@Melkiades Melkiades self-assigned this Mar 1, 2024
@edelarua edelarua marked this pull request as ready for review March 6, 2024 02:12
@edelarua edelarua requested a review from ayogasekaram as a code owner March 6, 2024 02:12
@edelarua
Copy link
Contributor Author

edelarua commented Mar 6, 2024

@Melkiades I have a rough solution here that is working for lists of listings (haven't tested lists of tables yet). Feel free to test and make any changes as you see fit! You can also modify insightsengineering/rlistings#166 if you'd like which is also working (in combination with this PR) to paginate by parameter.

EDIT: Full solution is now uploaded. :)

@edelarua edelarua requested a review from Melkiades March 7, 2024 00:33
R/mpf_exporters.R Outdated Show resolved Hide resolved
@Melkiades Melkiades changed the title Support pages by parameter for listings Support pagination of lists of rtables/rlistings objects Mar 22, 2024
Copy link
Contributor Author

@edelarua edelarua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm @Melkiades! Just a few suggestions to clarify the warnings.

R/mpf_exporters.R Outdated Show resolved Hide resolved
R/mpf_exporters.R Outdated Show resolved Hide resolved
R/mpf_exporters.R Outdated Show resolved Hide resolved
R/pagination.R Outdated Show resolved Hide resolved
Melkiades and others added 4 commits March 25, 2024 09:37
Co-authored-by: Emily de la Rua <59304861+edelarua@users.noreply.github.com>
Signed-off-by: Davide Garolini <dgarolini@gmail.com>
R/matrix_form.R Outdated Show resolved Hide resolved
R/pagination.R Outdated Show resolved Hide resolved
@shajoezhu shajoezhu requested a review from Melkiades April 1, 2024 14:32
Copy link
Contributor Author

@edelarua edelarua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is looking good as well! :)

@Melkiades Melkiades merged commit 82b1c8e into main Apr 3, 2024
25 checks passed
@Melkiades Melkiades deleted the 212_page_by_listings@main branch April 3, 2024 13:38
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
sme Tracks changes for the sme board
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow export_as_txt to process lists of listings
2 participants