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

The documentation of check_modules_datanames() #1395

Merged
merged 14 commits into from
Oct 29, 2024
Merged

Conversation

gogonzo
Copy link
Contributor

@gogonzo gogonzo commented Oct 25, 2024

Closes #1321
Little refurnishment:

  • splitted check_modules_datanames to two functions, one to return character and other to return shiny.tag.list
  • Renamed related utilities to better fit to what they do.

@gogonzo gogonzo added the core label Oct 25, 2024
@gogonzo gogonzo linked an issue Oct 25, 2024 that may be closed by this pull request
Copy link
Contributor

github-actions bot commented Oct 25, 2024

badge

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  ----------------------------------------------------------------------------------------------------------------------------------------
R/checkmate.R                        24       0  100.00%
R/dummy_functions.R                  47      11  76.60%   27, 29, 41, 52-59
R/get_rcode_utils.R                  12       0  100.00%
R/include_css_js.R                   22      17  22.73%   12-38, 76-82
R/init.R                            108      50  53.70%   107-114, 160-169, 171, 183-204, 229-232, 239-245, 248-249, 251
R/landing_popup_module.R             25      25  0.00%    61-87
R/module_bookmark_manager.R         158     127  19.62%   47-68, 88-138, 143-144, 156, 203, 238-315
R/module_data_summary.R             189      68  64.02%   24-52, 93, 187, 227-267
R/module_filter_data.R               64       2  96.88%   22-23
R/module_filter_manager.R           230      57  75.22%   56-62, 73-82, 90-95, 108-112, 117-118, 291-314, 340, 367, 379, 386-387
R/module_init_data.R                 74       0  100.00%
R/module_nested_tabs.R              236      91  61.44%   40-142, 174, 199-201, 320, 360
R/module_snapshot_manager.R         216     146  32.41%   89-95, 104-113, 121-133, 152-153, 170-180, 184-199, 201-208, 215-230, 234-238, 240-246, 249-262, 265-273, 304-318, 321-332, 335-341, 355
R/module_teal_data.R                149      10  93.29%   41-48, 84, 135-136
R/module_teal_lockfile.R            131      44  66.41%   32-36, 44-56, 59-61, 75, 85-87, 99-101, 109-118, 121, 123, 125-126, 160-161
R/module_teal_with_splash.R          12      12  0.00%    22-38
R/module_teal.R                     190      87  54.21%   48-143, 158, 184-185, 217
R/module_transform_data.R            54      32  40.74%   17-52
R/modules.R                         181      32  82.32%   173-177, 232-235, 333-334, 366-380, 418, 430-438
R/reporter_previewer_module.R        19       2  89.47%   30, 34
R/show_rcode_modal.R                 24      24  0.00%    17-42
R/tdata.R                            14      14  0.00%    19-61
R/teal_data_module-eval_code.R       24       0  100.00%
R/teal_data_module-within.R           7       0  100.00%
R/teal_data_module.R                 58       0  100.00%
R/teal_data_utils.R                  32       0  100.00%
R/teal_reporter.R                    68       6  91.18%   69, 77, 125-126, 129, 146
R/teal_slices-store.R                29       0  100.00%
R/teal_slices.R                      63       0  100.00%
R/TealAppDriver.R                   353     353  0.00%    52-735
R/utils.R                           182       0  100.00%
R/validate_inputs.R                  32       0  100.00%
R/validations.R                      58      37  36.21%   110-377
R/zzz.R                              15      11  26.67%   4-18
TOTAL                              3100    1258  59.42%

Diff against main

Filename                Stmts    Miss  Cover
--------------------  -------  ------  --------
R/module_teal_data.R       -3       0  -0.13%
R/utils.R                 -21       0  +100.00%
TOTAL                     -24       0  -0.31%

Results for commit: 302208d

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Oct 25, 2024

Unit Tests Summary

  1 files   25 suites   8m 39s ⏱️
263 tests 259 ✅ 4 💤 0 ❌
522 runs  518 ✅ 4 💤 0 ❌

Results for commit 302208d.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Oct 25, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-landing_popup 💔 $44.70$ $+1.19$ $0$ $0$ $0$ $0$
shinytest2-module_bookmark_manager 💔 $35.66$ $+1.25$ $0$ $0$ $0$ $0$
shinytest2-modules 💔 $38.58$ $+1.21$ $0$ $0$ $0$ $0$
shinytest2-reporter 💔 $66.93$ $+2.26$ $0$ $0$ $0$ $0$
shinytest2-teal_slices 💔 $61.21$ $+1.15$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
module_teal 💚 $18.21$ $-1.03$ creation_process_is_invoked_for_teal.lockfile.mode_enabled_and_snapshot_is_copied_to_teal_app.lock_and_removed_after_session_ended
module_teal 💀 $0.29$ $-0.29$ throws_warning_when_dataname_is_not_available
module_teal 👶 $+0.20$ warns_about_empty_data_when_none_of_module_datanames_is_available
module_teal 👶 $+0.23$ warns_about_empty_data_when_none_of_module_datanames_is_available_even_if_data_is_not_empty_
module_teal 👶 $+0.69$ warns_when_dataname_is_not_available
module_teal 👶 $+0.44$ warns_when_datanames_are_not_available

Results for commit 0a92441

♻️ This comment has been updated with latest results.

@gogonzo gogonzo changed the title 1321 docs improvement@main The documentation of check_modules_datanames() Oct 25, 2024
@averissimo averissimo self-assigned this Oct 25, 2024
Copy link
Contributor

@averissimo averissimo left a comment

Choose a reason for hiding this comment

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

Nice cleanup of the code! 👍

Just a few minor comments.

I tried to see if the recursive function would perform better as an iterative call, but they both run in the order of <1ms with the difference being negligible and attributed to the assertions (that are done over and over again)

R/utils.R Outdated Show resolved Hide resolved
R/utils.R Outdated Show resolved Hide resolved
R/utils.R Outdated Show resolved Hide resolved
R/utils.R Outdated Show resolved Hide resolved
Copy link
Contributor

@averissimo averissimo left a comment

Choose a reason for hiding this comment

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

Messages are not equivalent in the ending "Dataset available in data: ..."

I suggested the fix and recommend adding the test in the bottom (or similar)

pkgload::load_all("teal")
#> You are using teal version 0.15.2.9078
mods <- modules(example_module(datanames = c("iris", "mtcars", "co2")))

check_modules_datanames(mods, "iris")
#> [1] "Datasets \"mtcars\", \"co2\" are missing for module \"example teal module\". Datasets available in data: \"iris\""

check_modules_datanames_html(mods, "iris") |> toString() |> rvest::read_html() |> rvest::html_text() |> stringr::str_replace_all(" ?\n ?", "")
#> [1] " Datasets mtcars  and co2 are missing for tab 'example teal module'. No datasets are available in data."

Created on 2024-10-28 with reprex v2.1.1

testthat::test_that("check_modules_datanames result is equivalent in text and html format", {
  testthat::skip_if_not_installed("xml2")
  testthat::skip_if_not_installed("rvest")

  mods <- modules(
    example_module(datanames = c("iris", "mtcars", "co2"))
  )

  text_message <- check_modules_datanames(mods, "iris")
  html_message <- check_modules_datanames_html(mods, "iris")

  # Cleanup messages (making text_message the template/target)

  # remove quotes and backticks
  text_clean <- gsub("[\"'`]", "", text_message)
  html_clean <- gsub("[\"'`]", "", html_clean)

  # Cleanup extra spaces
  html_clean <- paste(
    collapse = " ",
    trimws(
      strsplit(
        rvest::html_text(rvest::minimal_html(html_message), trim = TRUE),
        "\n"
      )[[1]]
    )
  )

  # Replace " and " to ", " to be similar to text
  html_clean <- gsub(" and ", ", ", html_clean, fixed = TRUE)
  # Convert from UI's "tab" to warning's "module" to be similar to text warning
  html_clean <- gsub("are missing for tab", "are missing for module", html_clean, fixed = TRUE)
  # Replace singular to plural
  html_clean <- gsub("Dataset ", "Datasets ", html_clean)
  # Remove extra full stop in html
  html_clean <- gsub("[.]$", "", html_clean)

  # String should now be the same
  testthat::expect_identical(html_clean, text_clean)
})

R/utils.R Outdated Show resolved Hide resolved
R/utils.R Outdated Show resolved Hide resolved
R/utils.R Outdated Show resolved Hide resolved
Copy link
Contributor

@averissimo averissimo left a comment

Choose a reason for hiding this comment

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

Great work!

You made this manual conversion beautiful 💯

I left one **optional** (esthetics) suggestion that removes extra space before the commas.

R/utils.R Outdated Show resolved Hide resolved
@gogonzo gogonzo enabled auto-merge (squash) October 29, 2024 12:54
@gogonzo gogonzo merged commit b022241 into main Oct 29, 2024
28 checks passed
@gogonzo gogonzo deleted the 1321_docs_improvement@main branch October 29, 2024 13:31
@github-actions github-actions bot locked and limited conversation to collaborators Oct 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The documentation of check_modules_datanames() is incorrect
3 participants