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.

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

Fix subsetLiger removing cell.data and reporting 0 cells when no clustering available. #231

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

derpylz
Copy link

@derpylz derpylz commented Jul 5, 2021

Dear rliger team,

The current implementation of subsetLiger() deletes everything in the cell.data slot when subsetting a liger object that has no data in the clusters slot.

This is because of the following line:

liger/R/rliger.R

Line 6510 in 89b2443

a@cell.data <- droplevels(data.frame(object@cell.data[names(a@clusters), ]))

Here, the cell.data slot is subsetted by the names of a@cluster, which is of course empty if subsetLiger() is called before clustering.

It also causes the show method to report "0 total cells" for the subset.

This error can be very frustrating, as there are also no warnings.

It can be fixed by getting the cell names for the subsetting of the cell data e.g. from the raw.data slot. As in:

cell.names <- unname(unlist(lapply(a@raw.data, colnames)))
  if (ncol(a@cell.data) < ncol(object@cell.data)) {
    a@cell.data <- droplevels(data.frame(object@cell.data[cell.names, ]))
  }

I think should be a very small and easy change. This was already present in liger 0.5.0, and I had created a pull request also then. I've created this new pull request for the current version of rliger to make it easier to merge.

Best regards and thanks for the great package!
Nils

@cgao90
Copy link
Collaborator

cgao90 commented Oct 6, 2021

Hi Nils,

Very sorry about the late reply. We've incorporated your suggested fix on subsetLiger function. Thank you for identifying this issue and your effort to improve rliger!

Best,
Chao

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.

2 participants