Skip to content

[r] Rewrite LSI/binned feature selection to utilize new normalization functions, add DimReduction S3 Class#181

Closed
immanuelazn wants to merge 24 commits intobnprks:ia/feature-selectionfrom
immanuelazn:ia/lsi
Closed

[r] Rewrite LSI/binned feature selection to utilize new normalization functions, add DimReduction S3 Class#181
immanuelazn wants to merge 24 commits intobnprks:ia/feature-selectionfrom
immanuelazn:ia/lsi

Conversation

@immanuelazn
Copy link
Collaborator

@immanuelazn immanuelazn commented Jan 9, 2025

Details

This PR largely builds upon the previous PR from #156 for the LSI implementation. However, there are some primary changes to make it fit within the DimReduction framework noted in the design docs on #167.

Firstly an implementation of an S3 class DimReduction was made. DimReduction returns the results of a dimensionality reduction in attr cell_embeddings. Another attribute named fitted_params provides all the information required to re-run a dimensionality reduction, keeping all the fittings from the original data passed to create the class. fitted_params enables the project generic method, and can be built upon by all subclasses of DimReduction.

LSI implements both a subclass of DimReduction and provides a project method as well. The main fitted information utilized by a new run of project is the feature means when doing normalization, and the SVD params.

Tests are the same as in the previous PR, except with changes in syntax to reflect the new object styling

@immanuelazn immanuelazn changed the title [r] Rewrite LSI/binned feature selection to utilize new normalization functions [r] Rewrite LSI/binned feature selection to utilize new normalization functions, add DimReduction S3 Class Jan 9, 2025
#' @field cell_embeddings (IterableMatrix, dgCMatrix, matrix) The projected data
#' @field fitted_params (list) A list of parameters used for the transformation of a matrix.
#' @export
DimReduction <- function(x, fitted_params = list(), ...) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is the part that I am most iffy about. I don't see much in the codebase regarding new S3 classes, so I did not have much to reference off of. I am also iffy about how we want to expose the documentation to users. I created a new DimReduction field in pkgdown.yml to store this and project(). Any feedback on this section specifically would be greatly appreciated!

#' @return IterableMatrix object of the projected data.
#' @details DimReduction subclasses should use this to project new data with the same features, to project into the same latent space.
#' All required information to run a projection should be held in x$fitted_params, including pertinent parameters when construction the DimReduction subclass object.
#' If there are no rownames, assume that the matrix is in the same order as the original matrix, assuming that they have the same number of features.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The row matching logic intuitively makes sense to me to have, but I recognize this wasn't in the design docs. I was able to accomplish this in LSI() by providing an additional attr named feature_names. Should this be reflected in the DimReduction base class as well?

}


#' @export
Copy link
Collaborator Author

@immanuelazn immanuelazn Jan 9, 2025

Choose a reason for hiding this comment

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

Should I put a docstring for this or is this intuitive enough for S3 dispatch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant