Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Implement a new API #187

Closed
Tracked by #165 ...
gogonzo opened this issue Feb 3, 2023 · 8 comments · Fixed by #222
Closed
Tracked by #165 ...

Implement a new API #187

gogonzo opened this issue Feb 3, 2023 · 8 comments · Fixed by #222

Comments

@gogonzo
Copy link
Contributor

gogonzo commented Feb 3, 2023

Initial work for the #138

Check ?filter_state_api. There are set_filter_state, get_filter_state, remove_filter_state and get_filter_state. They are exported functions which calls methods of the same name in the FilteredData.
They passing the instructions to the classes to manage FilterState in the filter_state_list. Currently this communication is done by passing the list, for example

list(
  ADSL = list( # goes to FilteredDataset and to FilterStates
    AGE = list( # refers to FilterState for AGE column
      selected = , # initial selected values
      keep_na = , # initial value for keep_na
      keep_inf = , # initial value for keep_inf
    )
  ),
  MAE = list( # goes to MAEFilteredDataset
    subjects = list( # goes to MAEFIlterStates
       ...
    )
  )
)

We need to introduce a different way of passing this informations through the classes. Currently nested list shown above is transfered through FilteredData$set_filter_state -> FilteredDataset$set_filter_state -> FilterStates$set_filter_state -> FilterState$set_state. While passed from class to class list get unnested and FilterState.

We want to introduce something like this, which can be easily extended in the future by new type of filters or by new "arguments" which can cause some specific behavour of the filter-state within state_list.

filters = filter_settings(

    filter_var(dataname = "ADSL", varname = "ARM", choices = c("ARM A", "ARM B", "ARM C", NA)), # list
    
    filter_var(dataname = "ADSL", varname = "AGE", label = "AGE 18-60", choices = c(18, 60)),

    mae_parient_var(dataname = "MAE", varname = "gender"),

    mae_gene_subset(dataname = "MAE", experiment_name = "e1", varname = "gene.id"),

    mae_gene_select(),

   filterable = ...

  )

Comparing to the current API, in the new one we should be able also to specify "choices" which will limit number of choices in the initialized filter-state.


Arguments of the filter_var

  • @param dataname (character(1)) name of the dataset
  • @param varname (character(1)) name of the column in dataset
  • @param choices (vector) values of the column which should define range/set of the choices
  • @param selected (vector) values of the choices being a default/initial selection
  • @param label (character(1)) text describing the filter

Arguments of the filter_settings

  • @param ... (filter_var, mae_patient_var, mae_gene_subset, mae_gene_select)
  • @param filterable (list) named after datasets, containing character vectors containing columns which can be chosen to filter. Same as current implementation on main.

Because choices hasn't been yet implemented in the class it might be skipped if you found it difficult to just sneak-in in this PR.

@gogonzo
Copy link
Contributor Author

gogonzo commented Feb 28, 2023

Implementation should keep supporting current way of specification (using lists). Specifying a list instead of filter_settings should throw a warning saying specifying filters as lists is obsolete and will be deprecated in the next release. Please see documentation of ?set_filter_state to see...

@chlebowa
Copy link
Contributor

chlebowa commented Mar 6, 2023

Here is how I see this:

The structure is: FilteredData > FilteredDataset > FilterStates > FilterState.

filter_var creates an object that I will refer to as a teal_slice.

A teal_slice is a named list that has all information necessary to set a FilterState from any level in the structure. These mandatory elements are: dataname, varname, choices, selected, keep_na, keep_inf (optional), and fixed. All of these are stored in corresponding private fields in FilterState.

If a FilterState object has private$fixed == TRUE, none of its fields can be altered by any means. It can, however, be disabled or removed. (This links to #141). Such a FilterState has different (simpler) shiny UI and server functions.

A teal_slice can hold additional elements, that will be passed to ... in filter_var. All of these is stored in one field, private$extras in FilterState. private$extras cannot be modified.

filter_settings collates multiple teal_slices into a single unnamed list, teal_slices.
Within a teal_slices there is no hierarchy, each teal_slice is an equal element as it holds ALL information on a given FilterState.

Per @gogonzo's design, this allows things like Filter(function(x) x$dataname == "dataset1", teal_slices) to get filters pertinent to one data set. Some convenience functions can be built on this structure.

teal_slice and teal_slices are S3 classes to simplify argument checks and add some custom methods (e.g. print).

FilterState$get_state rebuilds and returns a teal_slice object based on current values of all appropriate fields.
FilterStates$get_states rebuilds and returns a teal_slices object based on current values of all FilterStates it contains.
FilteredDataset$get_filter_states rebuilds and returns a teal_slices from all FilterStates it contains.
FilteredData$get_filter_states rebuilds and returns a teal_slices from all FilteredDatasets it contains.
(some methods need renaming)

What do you think?

@chlebowa chlebowa linked a pull request Mar 6, 2023 that will close this issue
@chlebowa
Copy link
Contributor

chlebowa commented Mar 7, 2023

Once a FilterState has been initiated, these fields specified by filter_var cannot be modified:

  • dataname
  • varname
  • choices
  • fixed
    these fields can be modified:
  • selected
  • keep_na
  • keep_inf

If a FilterState is initiated with private$fixed = TRUE, none of the 8 fields can be modified.

@chlebowa
Copy link
Contributor

chlebowa commented Mar 9, 2023

The filterable argument in filter_settings: is it better to have it specify variables that can be filtered or values that can not be filtered? If the former, all variable names in a data set have to be typed when calling filter_settings as the call does not involve the data object in question. I suggest to have the argument specify "locked" variables and to have it default to character(0).

@gogonzo
Copy link
Contributor Author

gogonzo commented Mar 9, 2023

I suggest to have the argument specify "locked" variables and to have it default to character(0).

Could you elaborate more about locked? Is it alternative to "filterable"?

@chlebowa
Copy link
Contributor

chlebowa commented Mar 9, 2023

"locked" means non-filterable
I would suggest to have an argument like that rather than filterable (variables).

@gogonzo
Copy link
Contributor Author

gogonzo commented Mar 9, 2023

Agree, It makes more sense because by default all columns are filterable and we only substract from this set. But I suggest attribute name containing exclude phrase to describe this behaviour. locked seems to be closer to fixed, don't you think?

P.S. Feel free to remove set_filterable_varnames

@chlebowa
Copy link
Contributor

chlebowa commented Mar 9, 2023

Yes, "locked" was a placeholder name. I will go with "exclude".

BLAZEWIM added a commit that referenced this issue Mar 22, 2023
Should be reviewed together / with regard to
#222

Closes #232

Problems to solve:
- when initializing `FilterState` using FilterPanel API with choices,
when parameter choices limits the number of possible values to select
from, `is_any_filtered` should be set to TRUE to notice that the filter
is applied always, therefore it must appear in the call.
- test could not run due to different errors, mostly `argument
"dataname" missing`
- remove class `InteractiveFilterState` and `FilterState-abstract`, as
they are not needed anymore (fixed filter will be set using `fixed`
parameter)
```
state <- ChoicesFilterState$new(
  letters[c(1, 2)],
  x_reactive = reactive(NULL),
  "test",
  "var",
  choices = c("a", "b"),
  selected = c("a", "b"),
)
shiny::isolate(state$get_state())
state$.__enclos_env__$private$is_choice_limited

state <- ChoicesFilterState$new(
  letters,
  x_reactive = reactive(NULL),
  "test",
  "var",
  choices = c("a", "b"),
  selected = c("a", "b"),
)
shiny::isolate(state$get_state())
state$.__enclos_env__$private$is_choice_limited
```


# Pull Request

Part of #187

---------

Signed-off-by: Marek Blazewicz <110387997+BLAZEWIM@users.noreply.github.com>
Co-authored-by: Blazewicz <blazewim@emea.roche.com>
Co-authored-by: Dawid Kałędkowski <6959016+gogonzo@users.noreply.github.com>
Co-authored-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com>
chlebowa added a commit that referenced this issue Apr 14, 2023
This PR contains work towards a new filter panel API as outlined in
#187.

**TESTING class methods**:

1. preparations
```
# get FilteredData object on demand
utils::data(miniACC, package = "MultiAssayExperiment")
get_fd <- function() {
  init_filtered_data(
    x = list(
      iris = list(dataset = iris),
      mtcars = list(dataset = mtcars),
      mae = list(dataset = miniACC)
    )
  )
}


# specify filter states (old way)
fss <- list(
  iris = list(
    "Species" = list(selected = "setosa"),
    "Sepal.Length" = list(selected = c(5, 6))
  ),
  mtcars = list(
    "disp" = list(selected = c(0, 280)),
    "cyl" = list(selected = 6)
  ),
  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)
      )
    )
  )
)

# specify filter states (new way)
tss <- filter_settings(
  filter_var("iris", "Species", selected = "setosa"),
  filter_var("iris", "Sepal.Length", selected = c(5, 6)),
  filter_var("mtcars", "disp", selected = c(0, 280)),
  filter_var("mtcars", "cyl", selected = 6),
  filter_var("mae", "years_to_birth", selected = c(30, 50), keep_na = TRUE, keep_inf = FALSE, datalabel = "subjects", target = "y"),
  filter_var("mae", "vital_status", selected = "1", keep_na = FALSE, datalabel = "subjects", target = "y"),
  filter_var("mae", "gender", selected = "female", keep_na = TRUE, datalabel = "subjects", target = "y"),
  filter_var("mae", "ARRAY_TYPE", selected = "", keep_na = TRUE, datalabel = "RPPAArray", target = "subset")
)
```

2. convert old states specification to new one 
```
fss_new <- as.teal_slices(fss)
identical(fss, tss)
identical(fss_new, tss)
```
:point_up: this happens in `FilteredData$set_filter_state` with a
warning
Note `as.teal_slices` does not perform any validation, so a list-like
filter state that specifies filters on columns of `MAE@colData` that is
not wrapped as `list(MAE = list(...))` but is only `list(var =
list(...))` will be interpreted as a `data.frame` filter.

3. set states as list
```
# create FilteredData
fd <- get_fd()
# apply filter states
fd$set_filter_state(fss)
# see calls
fd$get_call("iris") %>% isolate
fd$get_call("mtcars") %>% isolate
fd$get_call("mae") %>% isolate
#recover filter states
fd$get_filter_state() %>% isolate
```

4. set states as `teal_slices`
```
# create FilteredData
fd <- get_fd()
# apply filter states
fd$set_filter_state(tss)
# see calls
fd$get_call("iris") %>% isolate
fd$get_call("mtcars") %>% isolate
fd$get_call("mae") %>% isolate
#recover filter states
fd$get_filter_state() %>% isolate
```
Note that calls are not generated. This is these filters are
instantiated and constructors don't know how to handle choices yet, so
by default they are created with everything selected, hence no calls.

5. modify states as `teal_slices`
```
fd$set_filter_state(tss)
# see calls
fd$get_call("iris") %>% isolate
fd$get_call("mtcars") %>% isolate
fd$get_call("mae") %>% isolate
#recover filter states
fd$get_filter_state() %>% isolate
```


**TESTING wrapper functions**:
```
datasets <- init_filtered_data(
  x = list(
    iris = list(dataset = iris),
    mae = list(dataset = miniACC)
  )
)
fs <- filter_settings(
  filter_var("iris", "Species", selected = c("setosa", "versicolor")),
  filter_var("iris", "Sepal.Length", selected = c(5.1, 6.4)),
  filter_var("mae", "years_to_birth", selected = c(30, 50),
             keep_na = TRUE, keep_inf = FALSE, datalabel = "subjects", target = "y"),
  filter_var("mae", "vital_status", selected = "1", keep_na = FALSE, datalabel = "subjects", target = "y"),
  filter_var("mae", "gender", selected = "female", keep_na = TRUE, datalabel = "subjects", target = "y"),
  filter_var("mae", "ARRAY_TYPE", selected = "", keep_na = TRUE, datalabel = "RPPAArray", target = "subset")
)

# set initial filter state
set_filter_state(datasets, filter = fs)

fd$.__enclos_env__$private$get_filter_count() %>% isolate

# get filter state
get_filter_state(datasets)

fd$.__enclos_env__$private$get_filter_count() %>% isolate

# modify filter state
set_filter_state(
  datasets,
  filter_settings(
    filter_var("iris", "Species", selected = "setosa", keep_na = TRUE)
  )
)

fd$.__enclos_env__$private$get_filter_count() %>% isolate

# remove specific filters
remove_filter_state(
  datasets,
  filter_settings(
    filter_var("iris", "Species"),
    filter_var("mae", "years_to_birth"),
    filter_var("mae", "vital_status")
  )
)

fd$.__enclos_env__$private$get_filter_count() %>% isolate

# remove all states
clear_filter_states(datasets)

fd$.__enclos_env__$private$get_filter_count() %>% isolate

```

---------

Signed-off-by: Marek Blazewicz <110387997+BLAZEWIM@users.noreply.github.com>
Co-authored-by: Marek Blazewicz <110387997+BLAZEWIM@users.noreply.github.com>
Co-authored-by: Blazewicz <blazewim@emea.roche.com>
Co-authored-by: Dawid Kałędkowski <6959016+gogonzo@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants