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

Allow custom formats, NA values in key columns #158

Merged
merged 11 commits into from Sep 29, 2023
Merged

Conversation

edelarua
Copy link
Contributor

Closes #157

@github-actions
Copy link
Contributor

github-actions bot commented Sep 27, 2023

badge

Code Coverage Summary

Filename                 Stmts    Miss  Cover    Missing
---------------------  -------  ------  -------  ---------------------------------------------------------
R/paginate_listing.R        40       1  97.50%   99
R/rlistings_methods.R      101      14  86.14%   39, 54, 58, 137-140, 143, 227-233
R/rlistings.R              159      20  87.42%   156-157, 160-161, 200-202, 366-369, 373-376, 405, 409-412
TOTAL                      300      35  88.33%

Diff against main

Filename         Stmts    Miss  Cover
-------------  -------  ------  -------
R/rlistings.R       +3       0  +0.24%
TOTAL               +3       0  +0.12%

Results for commit: 4e4fa2f

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Sep 27, 2023

Unit Tests Summary

  1 files    4 suites   5s ⏱️
27 tests 21 ✔️ 6 💤 0
71 runs  64 ✔️ 7 💤 0

Results for commit 52ecfd1.

♻️ This comment has been updated with latest results.

@edelarua edelarua marked this pull request as draft September 27, 2023 22:19
@edelarua edelarua changed the title Allow NA values in key columns Allow custom formats, NA values in key columns Sep 27, 2023
@edelarua edelarua marked this pull request as ready for review September 27, 2023 22:51
@edelarua edelarua mentioned this pull request Sep 27, 2023
42 tasks
@Melkiades Melkiades self-assigned this Sep 29, 2023
@Melkiades Melkiades added the bug Something isn't working label Sep 29, 2023
})

testthat::test_that("as_listing works with NA values in key cols", {
mtcars$gear[1:5] <- NA
Copy link
Contributor

Choose a reason for hiding this comment

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

I think all is very good. Only I found a case that I think we should trim, when a row is only NAs.

data(mtcars)
mtcars[2, ]
lsting <- as_listing(
    mtcars,
    key_cols = c("gear", "carb"),
    disp_cols = "qsec"
)

produces something that ends like:

        8     14.6 
 NA     NA     NA  

I think the lastline is not needed

Signed-off-by: Davide Garolini <davide.garolini@roche.com>
@edelarua edelarua merged commit 47b389a into main Sep 29, 2023
24 checks passed
@edelarua edelarua deleted the 157_key_col_nas@main branch September 29, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sme
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing values in key_cols under rlistings::as_listing()
2 participants