Use teal.data::get_code instead of teal.data:::get_code_dependency

[m7pr]

After introduction of ... in teal.data::get_code() during insightsengineering/teal.data#290,
I reckon there is no point using non-exported teal.data:::get_code_dependency
and we can just use exported teal.data::get_code.

Merge only when this is discussed and updated version of teal.data lands on CRAN insightsengineering/teal.data#291

[m7pr]

@chlebowa can you block review, untill insightsengineering/teal.data#291 is discussed and actions are taken upon it

[m7pr]

Review only after this is merged insightsengineering/teal.data#290

[github-actions]

Unit Tests Summary

  1 files   19 suites   11s ⏱️
208 tests 208 ✅ 0 💤 0 ❌
419 runs  419 ✅ 0 💤 0 ❌

Results for commit b23e1db.

♻️ This comment has been updated with latest results.

[averissimo]

Blocking as requested by @m7pr

Getting this in before {teal} is on CRAN would be nice to have 1 less getFromNamespace() in the package

[m7pr]

insightsengineering/teal.data#290 is merged and {teal.data} 0.5.0 is released on https://github.com/insightsengineering/teal.data/pull/297/files

I guess we can merge this one. Restarting GHA builds

[m7pr]

3 tests are failing

upstream package released fix

[m7pr]

I don't think this short change is enough.

teal.data:::get_code_dependency() took character as an input, however teal.data::get_code() takes teal_data object as an input (and extracts teal_data@code from the initial object).

Currently what gets inside get_datasets_code is "FilteredData" "R6".

[m7pr]

I think the quickest fix would be to create a character method for teal.data::get_code that would work directly on character code. So you could do as follows

• teal.data::get_code(teal_data()) # using method for teal_data object
• teal.data::get_code(teal_data()@code) # using method for character

[chlebowa]

method for teal.data::get_code that would work directly on character code

Bad idea.

Let's discuss on a call.

[m7pr]

Or we can change the order of in .datasets_to_data

  code <- c(
    get_rcode_str_install(),
    get_rcode_libraries(),
    get_datasets_code(datanames, datasets, hashes)
  )


  data <- do.call(
    teal.data::teal_data,
    args = c(data, code = list(code), join_keys = list(datasets$get_join_keys()[datanames]))
  )

So that teal.data::teal_data() is called and put into get_datasets_code(). Happy to discuss

[github-actions]

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------------------------------
R/dummy_functions.R                  30      21  30.00%   21-33, 36-43
R/get_rcode_utils.R                  31       1  96.77%   50
R/include_css_js.R                   22       0  100.00%
R/init.R                             86      31  63.95%   107-114, 160-161, 163, 175-196, 226-227, 229
R/landing_popup_module.R             25      25  0.00%    61-87
R/module_filter_manager.R           107      36  66.36%   37-43, 50-58, 67-72, 195, 200-213
R/module_nested_tabs.R              154      58  62.34%   39-112, 128, 180, 202, 228
R/module_snapshot_manager.R         209     157  24.88%   87-99, 127-136, 140-152, 154-161, 168-182, 186-188, 190-195, 198-208, 211-227, 236-251, 265-288, 291-302, 305-311, 325, 343-366
R/module_tabs_with_filters.R         76      33  56.58%   33-68, 100, 116
R/module_teal_with_splash.R         114       4  96.49%   110, 131, 197-198
R/module_teal.R                     106      29  72.64%   57, 68, 77, 150-151, 157, 176-207
R/modules.R                         153      27  82.35%   127-130, 147-151, 206-210, 292-293, 345, 357-365, 419-422
R/reporter_previewer_module.R        18       2  88.89%   30, 34
R/show_rcode_modal.R                 19      19  0.00%    17-36
R/tdata.R                            53       1  98.11%   154
R/teal_data_module-eval_code.R       27       0  100.00%
R/teal_data_module-within.R           7       0  100.00%
R/teal_data_module.R                  6       0  100.00%
R/teal_reporter.R                    62       5  91.94%   69, 118-119, 122, 139
R/teal_slices-store.R                29       0  100.00%
R/teal_slices.R                      59      12  79.66%   137-150
R/utils.R                           196      28  85.71%   117-144, 292
R/validate_inputs.R                  32       0  100.00%
R/validations.R                      58      37  36.21%   110-377
R/zzz.R                              11       7  36.36%   3-14
TOTAL                              1690     533  68.46%

Diff against main

Filename               Stmts    Miss  Cover
-------------------  -------  ------  -------
R/get_rcode_utils.R       -1       0  -0.10%
R/module_teal.R          -33      -3  -4.34%
R/utils.R                +49      +1  +4.08%
TOTAL                    +15      -2  +0.40%

Results for commit: b23e1db

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[chlebowa]

Should work 👍

[chlebowa]

Needs work after all.

May involve rewriting the module_datasets function or a completely different approach.

[m7pr]

Coolio. Just pasting the code we use to test the solution
(edited by @chlebowa)

app <- init(
  data = teal_data(
    IRIS = iris, 
    MTCARS = mtcars, 
    code = "IRIS <- iris; x <-5; MTCARS <- mtcars"
  ),
  modules = modules(
    example_module(label = "all datasets", datanames = "all"),
    example_module(label = "MTCARS only", datanames = "MTCARS"),
    example_module(label = "IRIS only", datanames = "IRIS")
  ),
  filter = teal_slices(
    teal_slice("MTCARS", "mpg"),
    teal_slice("IRIS", "Species"),
    module_specific = TRUE,
    mapping = list(
      global_filters = c("MTCARS mpg", "IRIS Species")
    )
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

[chlebowa]

It took me a while but:
I added a new file, modules_datasets.R. You can move the contents as you wish. The file contains

1. modules_structure - small functions that returns a nested list that reflects the structure of a teal_modules
2. modules_datasets - rather large function that returns list of FilteredData objects that reflects the shape of teal_modules
3. some testing, which can be easily adapted to the proper place in module_teal.R (uncomment)

[chlebowa]

I think utils.R makes some sense.

Tests for modules_structure - definitely.
Tests for modules_datasets - some are probably in order. You can use the commented "testing" code for those.

[chlebowa]

Or do you want me to do it? 😉

[m7pr]

@chlebowa sorry for the late reply. I can handle! No worries :)

[m7pr]

Hey @chlebowa I cleaned up your implementation and moved the function to utils.R. Added small roxygen documentation. Used your testing scenario in a test in testtthat folder. Lastly change the name of filter parameter, to filters so it plays with other parameters called filters in other functions that take teal_slices as an input.

[chlebowa]

Lastly change the name of filter parameter, to filters so it plays with other parameters called filters in other functions that take teal_slices as an input.

Excellent 👌

[github-actions]

Unit Test Performance Difference

Additional test case details

Test Suite	$Status$	Time on main	$±Time$	Test Case
utils	👶		$+0.17$	modules_datasets_returns_correct_structure

Results for commit 46845ce

♻️ This comment has been updated with latest results.

[chlebowa]

Some minor comments, otherwise ready.