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.

Sign up for GitHub

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

feat: added formatting function for filter panel classes #28

Merged
merged 12 commits into from
May 17, 2022
Merged

feat: added formatting function for filter panel classes #28

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3020d22
feat: added a method to convert filter state to string
kpagacz May 5, 2022
e37e73c
wip
kpagacz May 5, 2022
a0ab58b
wip
kpagacz May 5, 2022
5d2986f
wip
kpagacz May 10, 2022
49ab696
wip
kpagacz May 10, 2022
0ede485
feat: added human-readable string represtations of filtering classes
kpagacz May 12, 2022
ee4f770
Apply suggestions from code review
kpagacz May 13, 2022
7e62953
review no 1
kpagacz May 13, 2022
8205487
fixes
kpagacz May 13, 2022
967a751
added timezone to posixct constructors
kpagacz May 13, 2022
02d3b9c
Apply suggestions from code review
kpagacz May 17, 2022
99999c0
Merge branch 'main' into 8_teal.reporter_filter_list_print2@main
kpagacz May 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 81 additions & 2 deletions R/FilterState.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,25 @@ FilterState <- R6::R6Class( # nolint
return(invisible(NULL))
},

#' @description
#' Returns a formatted string representing this `FilterState`.
#'
#' @param indent (`numeric(1)`) the number of spaces before after each new line character of the formatted string.
#' Default: 0
#' @return `character(1)` the formatted string
#'
format = function(indent = 0) {
checkmate::assert_number(indent, finite = TRUE, lower = 0)

sprintf(
"%sFiltering on: %s\n%1$s Selected values: %s\n%1$s Include missing values: %s",
format("", width = indent),
self$get_varname(deparse = TRUE),
paste0(format(self$get_selected(), nsmall = 3), collapse = " "),
format(self$get_keep_na())
)
},

#' @description
#' Returns reproducible condition call for current selection relevant
#' for selected variable type.
Expand All @@ -370,7 +389,7 @@ FilterState <- R6::R6Class( # nolint
#' @return (`name` or `character(1)`)
get_dataname = function(deparse = TRUE) {
if (isTRUE(deparse)) {
deparse(private$input_dataname)
deparse1(private$input_dataname)
} else {
private$input_dataname
}
Expand All @@ -397,7 +416,7 @@ FilterState <- R6::R6Class( # nolint
#' @return (`name` or `character(1)`)
get_varname = function(deparse = FALSE) {
if (isTRUE(deparse)) {
deparse(private$varname)
deparse1(private$varname)
} else {
private$varname
}
Expand Down Expand Up @@ -1271,6 +1290,26 @@ RangeFilterState <- R6::R6Class( # nolint
return(invisible(self))
},

#' @description
#' Returns a formatted string representing this `LogicalFilterState`.
#'
#' @param indent (`numeric(1)`) the number of spaces before after each new line character of the formatted string.
#' Default: 0
#' @return `character(1)` the formatted string
#'
format = function(indent = 0) {
checkmate::assert_number(indent, finite = TRUE, lower = 0)

sprintf(
"%sFiltering on: %s\n%1$s Selected range: %s - %s\n%1$s Include missing values: %s",
format("", width = indent),
self$get_varname(deparse = TRUE),
format(self$get_selected(), nsmall = 3)[1],
format(self$get_selected(), nsmall = 3)[2],
format(self$get_keep_na())
)
Comment on lines +1311 to +1318
Copy link
Contributor

Choose a reason for hiding this comment

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

RangeFilterState handles also Inf

Suggested change
sprintf(
"%sFiltering on: %s\n%1$s Selected range: %s - %s\n%1$s Include missing values: %s",
format("", width = indent),
self$get_varname(deparse = TRUE),
format(self$get_selected(), nsmall = 3)[1],
format(self$get_selected(), nsmall = 3)[2],
format(self$get_keep_na())
)
sprintf(
"%sFiltering on: %s\n%1$s Selected range: %s - %s\n%1$s Include missing values: %s\n%1$s Include infinite values: %s",
format("", width = indent),
self$get_varname(deparse = TRUE),
format(self$get_selected(), nsmall = 3)[1],
format(self$get_selected(), nsmall = 3)[2],
format(self$get_keep_na()),
format(self$get_keep_inf())
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's consequential for a human readable output to be honest. It was a conscious decision.

},

#' @description
#' Answers the question of whether the current settings and values selected actually filters out any values.
#' @return logical scalar
Expand Down Expand Up @@ -2048,6 +2087,26 @@ DateFilterState <- R6::R6Class( # nolint
return(invisible(self))
},

#' @description
#' Returns a formatted string representing this `DateFilterState`.
#'
#' @param indent (`numeric(1)`) the number of spaces before after each new line character of the formatted string.
#' Default: 0
#' @return `character(1)` the formatted string
#'
format = function(indent = 0) {
checkmate::assert_number(indent, finite = TRUE, lower = 0)

sprintf(
"%sFiltering on: %s\n%1$s Selected range: %s - %s\n%1$s Include missing values: %s",
format("", width = indent),
self$get_varname(deparse = TRUE),
format(self$get_selected(), nsmall = 3)[1],
format(self$get_selected(), nsmall = 3)[2],
kpagacz marked this conversation as resolved.
Show resolved Hide resolved
format(self$get_keep_na())
)
},

#' @description
#' Answers the question of whether the current settings and values selected actually filters out any values.
#' @return logical scalar
Expand Down Expand Up @@ -2346,6 +2405,26 @@ DatetimeFilterState <- R6::R6Class( # nolint
return(invisible(self))
},

#' @description
#' Returns a formatted string representing this `DatetimeFilterState`.
#'
#' @param indent (`numeric(1)`) the number of spaces before after each new line character of the formatted string.
#' Default: 0
#' @return `character(1)` the formatted string
#'
format = function(indent = 0) {
checkmate::assert_number(indent, finite = TRUE, lower = 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

should these be assert_integerish ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The whole method doesn't throw even when passed a fraction but good point. Probably yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's because format implicitly casts width to an integer XD


sprintf(
"%sFiltering on: %s\n%1$s Selected range: %s - %s\n%1$s Include missing values: %s",
format("", width = indent),
self$get_varname(deparse = TRUE),
format(self$get_selected(), nsmall = 3)[1],
format(self$get_selected(), nsmall = 3)[2],
format(self$get_keep_na())
)
},

#' @description
#' Answers the question of whether the current settings and values selected actually filters out any values.
#' @return logical scalar
Expand Down
124 changes: 111 additions & 13 deletions R/FilterStates.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,24 @@ FilterStates <- R6::R6Class( # nolint
logger::log_trace("Instantiated { class(self)[1] }, dataname: { deparse1(private$input_dataname) }")
invisible(self)
},

#' @description
#' Returns the input dataname
#' @return (`character(1)`) the input dataname
kpagacz marked this conversation as resolved.
Show resolved Hide resolved
get_datalabel = function() {
private$datalabel
},

#' @description
#' Returns the formatted string representing this `FilterStates` object.
#'
#' @param indent (`numeric(1)`) the number of spaces before each line of the representation
#' @return `character(1)` the formatted string
#'
format = function(indent) {
stop("Pure virtual method")
},

#' @description
#' Filter call
#'
Expand Down Expand Up @@ -647,14 +665,15 @@ FilterStates <- R6::R6Class( # nolint
)
)

#' Specialization of `FilterStates` for a base `data.frame`.
#' @title DFFFilterStates
#' @description Specialization of `FilterStates` for a base `data.frame`.
#'
#' @keywords internal
DFFilterStates <- R6::R6Class( # nolint
classname = "DFFilterStates",
inherit = FilterStates,
public = list(
#' Initializes `DFFilterStates` object
#' @description Initializes `DFFilterStates` object
#'
#' Initializes `DFFilterStates` object by setting `input_dataname`,
#' `output_dataname` and initializing `ReactiveQueue`. This class contains a
Expand Down Expand Up @@ -689,6 +708,22 @@ DFFilterStates <- R6::R6Class( # nolint
)
},

#' @description
#' Returns the formatted string representing this `FilterStates` object.
#'
#' @param indent (`numeric(1)`) the number of spaces before each line of the representation
#' @return `character(1)` the formatted string
format = function(indent = 0) {
checkmate::assert_number(indent, finite = TRUE, lower = 0)

formatted_states <- vapply(
self$queue_get(1L), function(state) state$format(indent = indent),
USE.NAMES = FALSE, FUN.VALUE = character(1)
)
paste(formatted_states, collapse = "\n")
},

#' @description
#' Get function name
#'
#' Get function name used to create filter call.
Expand Down Expand Up @@ -1041,13 +1076,14 @@ DFFilterStates <- R6::R6Class( # nolint
)


#' Specialization of `FilterStates` for `MultiAssayExperiment`.
#' @title MAEFilterStates
#' @description Specialization of `FilterStates` for `MultiAssayExperiment`.
#' @keywords internal
MAEFilterStates <- R6::R6Class( # nolint
classname = "MAEFilterStates",
inherit = FilterStates,
public = list(
#' Initialize `MAEFilterStates` object
#' @description Initializes `MAEFilterStates` object
#'
#' Initialize `MAEFilterStates` object
#'
Expand Down Expand Up @@ -1082,11 +1118,24 @@ MAEFilterStates <- R6::R6Class( # nolint
return(invisible(self))
},

#' Get function name
#' @description
#' Returns the formatted string representing this `MAEFilterStates` object.
#'
#' Get function name used to create filter call.
#' For `MAEFilterStates`
#' `MultiAssayExperiment::subsetByColData` is used.
#' @param indent (`numeric(1)`) the number of spaces before each line of the representation
#' @return `character(1)` the formatted string
format = function(indent = 0) {
checkmate::assert_number(indent, finite = TRUE, lower = 0)

if (length(self$queue_get(1L)) > 0) {
formatted_states <- sprintf("%sSubject filters:", format("", width = indent))
for (state in self$queue_get(1L)) formatted_states <- c(formatted_states, state$format(indent = indent + 2))
paste(formatted_states, collapse = "\n")
}
},

#' @description
#' Returns function name used to create filter call.
#' For `MAEFilterStates` `MultiAssayExperiment::subsetByColData` is used.
#' @return `character(1)`
get_fun = function() {
return("MultiAssayExperiment::subsetByColData")
Expand Down Expand Up @@ -1195,7 +1244,8 @@ MAEFilterStates <- R6::R6Class( # nolint
NULL
},

#' @description Remove a variable from the `ReactiveQueue` and its corresponding UI element.
#' @description
#' Removes a variable from the `ReactiveQueue` and its corresponding UI element.
#'
#' @param element_id (`character(1)`)\cr name of `ReactiveQueue` element.
#'
Expand Down Expand Up @@ -1403,13 +1453,14 @@ MAEFilterStates <- R6::R6Class( # nolint
)
)

#' Specialization of `FilterStates` for `SummaryExperiment`.
#' @title SEFilterStates
#' @description Specialization of `FilterStates` for `SummaryExperiment`.
#' @keywords internal
SEFilterStates <- R6::R6Class( # nolint
classname = "SEFilterStates",
inherit = FilterStates,
public = list(
#' Initialize `SEFilterStates` object
#' @description Initialize `SEFilterStates` object
#'
#' Initialize `SEFilterStates` object
#'
Expand All @@ -1435,6 +1486,36 @@ SEFilterStates <- R6::R6Class( # nolint
)
},

#' @description
#' Returns the formatted string representing this `MAEFilterStates` object.
#'
#' @param indent (`numeric(1)`) the number of spaces before each line of the representation
#' @return `character(1)` the formatted string
format = function(indent = 0) {
checkmate::assert_number(indent, finite = TRUE, lower = 0)

whitespace_indent <- format("", width = indent)
formatted_states <- c()
if (!is.null(self$queue_get(queue_index = "subset"))) {
formatted_states <- c(formatted_states, paste0(whitespace_indent, " Subsetting:"))
for (state in self$queue_get(queue_index = "subset")) {
formatted_states <- c(formatted_states, state$format(indent = indent + 4))
}
}

if (!is.null(self$queue_get(queue_index = "select"))) {
formatted_states <- c(formatted_states, paste0(whitespace_indent, " Selecting:"))
for (state in self$queue_get(queue_index = "select")) {
formatted_states <- c(formatted_states, state$format(indent = indent + 4))
}
}

if (length(formatted_states) > 0) {
formatted_states <- c(paste0(whitespace_indent, "Assay ", self$get_datalabel(), " filters:"), formatted_states)
paste(formatted_states, collapse = "\n")
}
},

#' @description
#' Server module
#' @param id (`character(1)`)\cr
Expand Down Expand Up @@ -1942,13 +2023,14 @@ SEFilterStates <- R6::R6Class( # nolint
)
)

#' Specialization of `FilterStates` for a base matrix.
#' @title MatrixFilterStates
#' @description Specialization of `FilterStates` for a base matrix.
#' @keywords internal
MatrixFilterStates <- R6::R6Class( # nolint
classname = "MatrixFilterStates",
inherit = FilterStates,
public = list(
#' Initialize `MatrixFilterStates` object
#' @description Initialize `MatrixFilterStates` object
#'
#' Initialize `MatrixFilterStates` object
#'
Expand All @@ -1970,6 +2052,22 @@ MatrixFilterStates <- R6::R6Class( # nolint
)
},

#' @description
#' Returns the formatted string representing this `MatrixFilterStates` object.
#'
#' @param indent (`numeric(1)`) the number of spaces before each line of the representation
#' @return `character(1)` the formatted string
format = function(indent = 0) {
checkmate::assert_number(indent, finite = TRUE, lower = 0)

formatted_states <- c()
whitespace_indent <- paste0(rep(" ", indent), collapse = "")
kpagacz marked this conversation as resolved.
Show resolved Hide resolved
for (state in self$queue_get(queue_index = "subset")) {
formatted_states <- c(formatted_states, state$format(indent = indent + 2))
}
paste(formatted_states, collapse = "\n")
},

#' @description
#' Server module
#' @param id (`character(1)`)\cr
Expand Down
36 changes: 34 additions & 2 deletions R/FilteredData.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ FilteredData <- R6::R6Class( # nolint
names(private$filtered_datasets)
},


#' Gets data label for the dataset
#'
#' Useful to display in `Show R Code`.
Expand Down Expand Up @@ -315,7 +314,6 @@ FilteredData <- R6::R6Class( # nolint
intersect(self$datanames(), datanames)
},


#' @description
#' Adds a `TealDataset` object to this `FilteredData`
#'
Expand Down Expand Up @@ -377,6 +375,40 @@ FilteredData <- R6::R6Class( # nolint
Filter(function(x) length(x) > 0, states)
},

#' @description
#' Returns the filter state formatted for printing to an `IO` device.
#'
#' @return `character` the pre-formatted filter state
#' @examples
#' datasets <- teal.slice:::FilteredData$new()
#' datasets$set_dataset(teal.data::dataset("iris", iris))
#' utils::data(miniACC, package = "MultiAssayExperiment")
#' datasets$set_dataset(teal.data::dataset("mae", miniACC))
#' fs <- list(
#' iris = list(
#' Sepal.Length = list(selected = c(5.1, 6.4), keep_na = TRUE, keep_inf = FALSE),
#' Species = list(selected = c("setosa", "versicolor"), keep_na = FALSE)
#' ),
#' mae = list(
#' subjects = list(
#' years_to_birth = list(selected = c(30, 50), keep_na = TRUE, keep_inf = FALSE),
#' vital_status = list(selected = "1", keep_na = FALSE),
#' gender = list(selected = "female", keep_na = TRUE)
#' ),
#' RPPAArray = list(
#' subset = list(ARRAY_TYPE = list(selected = "", keep_na = TRUE))
#' )
#' )
#' )
#' datasets$set_filter_state(state = fs)
#' cat(shiny::isolate(datasets$get_formatted_filter_state()))
#'
get_formatted_filter_state = function() {
out <- c()
for (filtered_dataset in self$get_filtered_dataset()) out <- c(out, filtered_dataset$get_formatted_filter_state())
Copy link
Contributor

@nikolas-burkoff nikolas-burkoff May 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

wouldn't

out <- vapply(self$get_filtered_dataset(), 
  function(x) x$get_formatted_filter_state(), FUN.VALUE = character(1)
)
paste(out, collapse = "\n")

work as well? And in other places? Or is it a deliberate decision to use for loops

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They don't return a character(1) always.

Copy link
Contributor Author

@kpagacz kpagacz May 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Plus, I usually append things to the out object (not always) before looping and apply stuff doesn't capture by reference from their lexical scopes, so I opted to use for loops everywhere. Plus, for loops in R stopped being less performant than apply stuff ages ago, so tbh there's no particularly good reason to use them except for 'list or dict comprehension'-style programming.

Copy link
Contributor

Choose a reason for hiding this comment

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

fair enough just askin' ;)

paste(out, collapse = "\n")
},

#' @description
#' Sets active filter states.
#' @param state (`named list`)\cr
Expand Down
Loading