microbiome / mia

Microbiome analysis
https://microbiome.github.io/mia/
Artistic License 2.0
46 stars 25 forks source link

MARGIN and altexp parameters #480

Open TuomasBorman opened 6 months ago

TuomasBorman commented 6 months ago

Certain functions do not yet support MARGIN and altexp parameters. MARGIN is for specifying rows or columns, and altexp is for specifying alternative experiment from altExp slot.

We have decided to not limit functions to work only in row or column-wise. Allowing functions to work both row and column-wise might help future maintenance. Additional benefit for this approach is additional generalization; functions work more similarly using general internal functions. Moreover, there could be an application where colTree is needed but we do not really support colTrees yet in our functions.

Proposal is the following structure:

addValues <- function(tse, ...){
    # Get assay
    mat <- .check_and_get_assay(tse, ...)
    # Calculate some values
    result <- calculate(mat)
    # Store values to colData
    tse <- .add_values_to_colData(tse, result, ...)
}

.check_and_get_assay <- function(tse, MARGIN = 1, altexp = NULL, ...){
    # Do something
}

.add_values_to_colData <- function(tse, values, MARGIN = 2, altexp = NULL, ...){
    # Do something
}

I made a draft to branch https://github.com/microbiome/mia/tree/getter_and_setters --> Work with this branch.

See utils.R and estimateDiversity.R --> abundance matrix and trees are now fetched with generic getters from utils.R. Also the result is stored to colData with this generic setter.

Tasks to do:

  1. Modify functions so that they fetch and store the values by using these generic functions from utils.R. No need to add visible MARGIN and altexp parameter, we can add them later if we think they are good to have.
  2. Check that the functions work when MARGIN and altexp is specified.
  3. Report any possible problems (there might be in some cases. For instance columns do not have referenceSeq slot as rows have. Also, estimateDIversity(tse, MARGIN = 2, index = "fisher") is not working but the issue is not in our methods.)
Daenarys8 commented 5 months ago

I added a commit, some initial feedback would help speed up the changes to be added.