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

fixes the relationship between module_specific and mapping. #968

Closed
wants to merge 5 commits into from

Conversation

kartikeyakirar
Copy link
Contributor

fixes: #966

Example

# module_specific is not specified
teal_slices(
    # filters created with id
    teal_slice(dataname = "mtcars", varname = "mpg", id = "filter 1"),
    teal_slice(dataname = "mtcars", varname = "cyl", id = "filter 2"),
    teal_slice(dataname = "mtcars", varname = "disp", id = "filter 3"),
    teal_slice(dataname = "mtcars", varname = "hp", id = "filter 4"),
    teal_slice(dataname = "mtcars", varname = "drat", id = "filter 5"),
    teal_slice(dataname = "mtcars", varname = "wt", id = "filter 6"),
    
    # filters mapped to modules
    mapping = list(
        "module 1" = c("filter 2", "filter 4"),
        "module 2" = "filter 3",
        "module 3" = "filter 2",
        global_filters = "filter 1"
    )
)
############output############
filter
{
  "slices": [
    {
      "dataname"       : "mtcars",
      "varname"        : "mpg",
      "id"             : "filter 1",
      "fixed"          : false,
      "anchored"       : false,
      "multiple"       : true
    },
    {
      "dataname"       : "mtcars",
      "varname"        : "cyl",
      "id"             : "filter 2",
      "fixed"          : false,
      "anchored"       : false,
      "multiple"       : true
    },
    {
      "dataname"       : "mtcars",
      "varname"        : "disp",
      "id"             : "filter 3",
      "fixed"          : false,
      "anchored"       : false,
      "multiple"       : true
    },
    {
      "dataname"       : "mtcars",
      "varname"        : "hp",
      "id"             : "filter 4",
      "fixed"          : false,
      "anchored"       : false,
      "multiple"       : true
    },
    {
      "dataname"       : "mtcars",
      "varname"        : "drat",
      "id"             : "filter 5",
      "fixed"          : false,
      "anchored"       : false,
      "multiple"       : true
    },
    {
      "dataname"       : "mtcars",
      "varname"        : "wt",
      "id"             : "filter 6",
      "fixed"          : false,
      "anchored"       : false,
      "multiple"       : true
    }
  ],
  "attributes": {
    "allow_add"        : true,
    "mapping"          : {
      "module 1"       : ["filter 2", "filter 4"],
      "module 2"       : "filter 3",
      "module 3"       : "filter 2",
      "global_filters" : "filter 1"
    },
    "module_specific"  : true
  }
} 

Copy link
Contributor

badge

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------------------------------
R/dummy_functions.R                 92      50  45.65%   9-71
R/get_rcode_utils.R                 46       1  97.83%   49
R/include_css_js.R                  24       0  100.00%
R/init.R                            79      25  68.35%   143, 168-189, 221-223, 225-227, 229-230
R/landing_popup_module.R            25      25  0.00%    61-87
R/module_filter_manager.R          107      29  72.90%   62-70, 79-84, 228, 233-246
R/module_nested_tabs.R             176      14  92.05%   72, 119, 138-145, 163, 216, 238, 271
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, 346-369
R/module_tabs_with_filters.R        73       2  97.26%   95, 140
R/module_teal_with_splash.R        119      11  90.76%   73, 83-91, 122
R/module_teal.R                    141       8  94.33%   68, 71, 158-159, 165, 195, 203-204
R/modules_debugging.R               18      18  0.00%    25-44
R/modules.R                        143      26  81.82%   119, 132, 226-229, 243-248, 259-263, 378-421
R/reporter_previewer_module.R       18       2  88.89%   26, 30
R/show_rcode_modal.R                20      20  0.00%    16-37
R/tdata.R                           39       1  97.44%   158
R/teal_data_module.R                 6       0  100.00%
R/teal_reporter.R                   60       5  91.67%   65, 116-117, 120, 137
R/teal_slices-store.R               25       0  100.00%
R/teal_slices.R                     66      12  81.82%   147-160
R/utils.R                          110      27  75.45%   113-140
R/validate_inputs.R                 32       0  100.00%
R/validations.R                     58      37  36.21%   109-371
R/zzz.R                             11       7  36.36%   3-14
TOTAL                             1697     477  71.89%

Diff against main

Filename           Stmts    Miss  Cover
---------------  -------  ------  -------
R/teal_slices.R       +7       0  +2.16%
TOTAL                 +7       0  +0.12%

Results for commit: 54b8324

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

Unit Tests Summary

    1 files    18 suites   12s ⏱️
197 tests 197 ✔️ 0 💤 0
374 runs  374 ✔️ 0 💤 0

Results for commit 54b8324.

Copy link
Contributor

Unit Test Performance Difference

Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
module_teal 👶 $+0.03$ srv_teal_initialized_data_containing_same_FilteredData_when_module_specific_is_not_specified
teal_slices 👶 $+0.02$ teal_slices_dynamically_calculates_module_specific

Results for commit 6275ace

♻️ This comment has been updated with latest results.

#' If `module_specific` is not provided, its value is inferred based on the `mapping` parameter:
#' - If `mapping` is provided and non-empty, `module_specific` is set to `TRUE`.
#' - If `mapping` is missing or empty, `module_specific` defaults to `FALSE`.
#' This behavior allows for dynamic determination of filter panel specificity based on the provided `mapping`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#' This behavior allows for dynamic determination of filter panel specificity based on the provided `mapping`.

@@ -14,6 +14,11 @@
#' of filters specified - see `mapping` argument.
#' - `FALSE` when one filter panel needed to all modules. All filters will be shared
#' by all modules.
#' Note: This parameter does not have a default value and should be explicitly provided.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#' Note: This parameter does not have a default value and should be explicitly provided.

Comment on lines +18 to +20
#' If `module_specific` is not provided, its value is inferred based on the `mapping` parameter:
#' - If `mapping` is provided and non-empty, `module_specific` is set to `TRUE`.
#' - If `mapping` is missing or empty, `module_specific` defaults to `FALSE`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#' If `module_specific` is not provided, its value is inferred based on the `mapping` parameter:
#' - If `mapping` is provided and non-empty, `module_specific` is set to `TRUE`.
#' - If `mapping` is missing or empty, `module_specific` defaults to `FALSE`.
#' Defaults to `TRUE` if `mapping` provided, otherwise to `FALSE`.

@@ -5,6 +5,7 @@
* `data` argument in `init` now accepts `teal_data` and `teal_data_module`.
* Added `landing_popup_module` function which creates a module that will display a popup when the app starts. The popup will block access to the app until it is dismissed.
* Filter state snapshots can now be uploaded from file. See `?snapshot`.
* Removed default value for `module_specific` from `teal_slices`, now dynamically determined based on mapping content—if mapping is provided and non-empty, `module_specific` defaults to `TRUE`, otherwise to `FALSE`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Removed default value for `module_specific` from `teal_slices`, now dynamically determined based on mapping content—if mapping is provided and non-empty, `module_specific` defaults to `TRUE`, otherwise to `FALSE`.
* Removed default value for `module_specific` argument in `teal_slices`. The default value now depends on the `mapping` argument.

@kartikeyakirar
Copy link
Contributor Author

closing this PR based on comment #966 (comment).

@kartikeyakirar kartikeyakirar deleted the 966_fixing_mapping@main branch November 21, 2023 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The mapping in teal_slice is lost
2 participants