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
duplicated available slices #377
duplicated available slices #377
Conversation
Code Coverage Summary
Diff against main
Results for commit: 6166519 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Code Coverage Summary
Diff against main
Results for commit: 8328234 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
@@ -482,7 +482,7 @@ FilteredData <- R6::R6Class( # nolint | |||
private$module_add <- module_add | |||
} | |||
|
|||
lapply(datanames, function(dataname) { | |||
lapply(self$datanames(), function(dataname) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess these lines above can be removed now:
datanames <- slices_field(state, "dataname")
checkmate::assert_subset(datanames, self$datanames())
R/FilteredData.R
Outdated
is_not_expr <- vapply(private$available_teal_slices(), inherits, logical(1), "teal_slice_expr") | ||
slice_reference[is_duplicated_reference & is_active & !is_not_expr] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_not_expr <- vapply(private$available_teal_slices(), inherits, logical(1), "teal_slice_expr") | |
slice_reference[is_duplicated_reference & is_active & !is_not_expr] | |
is_not_expr <- !vapply(private$available_teal_slices(), inherits, logical(1), "teal_slice_expr") | |
slice_reference[is_duplicated_reference & is_active & is_not_expr] |
R/FilteredData.R
Outdated
interactive_choice_mock <- lapply( | ||
slices_interactive(), | ||
function(slice) { | ||
# we need to isolate changes in the fields of the slice (teal_slice) | ||
shiny::isolate({ | ||
checkbox_group_element( | ||
name = session$ns("available_slices_id"), | ||
value = slice$id, | ||
label = slice$id, | ||
checked = if (slice$id %in% active_slices_ids) "checked", | ||
disabled = slice$locked | ||
disabled = slice$locked || | ||
get_default_slice_id(slice) %in% duplicated_slice_refs && | ||
!slice$id %in% active_slices_ids | ||
) | ||
} | ||
) | ||
}) | ||
} | ||
) | ||
|
||
non_interactive_choice_mock <- lapply( | ||
slices_fixed(), | ||
function(slice) { | ||
non_interactive_choice_mock <- lapply( | ||
slices_fixed(), | ||
function(slice) { | ||
# we need to isolate changes in the fields of the slice (teal_slice) | ||
isolate( | ||
checkbox_group_element( | ||
name = session$ns("available_slices_id"), | ||
value = slice$id, | ||
label = slice$id, | ||
checked = if (slice$id %in% active_slices_ids) "checked", | ||
disabled = slice$locked | ||
disabled = slice$locked || | ||
get_default_slice_id(slice) %in% duplicated_slice_refs && | ||
!slice$id %in% active_slices_ids | ||
) | ||
} | ||
) | ||
) | ||
} | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps assign the function(slice)
for cleaner code?
R/FilteredData.R
Outdated
tags$strong("Fixed filters"), | ||
non_interactive_choice_mock, | ||
tags$strong("Iteractive filters"), | ||
interactive_choice_mock, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
R/FilteredDatasetDefault.R
Outdated
lapply(state, function(slice) { | ||
checkmate::assert_true(slice$dataname == private$dataname) | ||
}) | ||
checkmate::assert_class(state, "teal_slices") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lapply(state, function(slice) { | |
checkmate::assert_true(slice$dataname == private$dataname) | |
}) | |
checkmate::assert_class(state, "teal_slices") | |
checkmate::assert_class(state, "teal_slices") | |
lapply(state, function(slice) { | |
checkmate::assert_true(slice$dataname == private$dataname) | |
}) |
Closes #374
Fixes problem when adding new filter based on the variable for which there is already filter in
available_teal_slices
. Filters duplicated are disabled from the selection. Filter is disabled when:dataname
,varname
,experiment
andarg
. If there is any filter which has exactly the same values in all of these fields then it is duplpicated.Fixed in this PR:
$set_filter_states
,teal_slice
attributes were changed only for datasets which hadteal_slice
object - loop was done on slices-datasets instead of all datasets.character
. This also matches logic instate_list_add
which also accepts multiple elements.