-
Notifications
You must be signed in to change notification settings - Fork 0
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
iLISI function #95
iLISI function #95
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good! Just a few minor comments.
Co-authored-by: Ally Hawkins <[email protected]>
For the final columns, I ended up keeping all of the following: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just one minor error I think
Co-authored-by: Ally Hawkins <[email protected]>
Yeah I did a silly thing here merging-wise, sorry @jashapiro |
@allyhawkins Silly things happened with merging here which mean I'll have to re-file this PR to |
Closes #89
Stacked on #92
This PR adds the file
scripts/utils/calculate-iLISI.R
to calculate iLISI scores. I also added this function to the testing script for testing. Along the way, I ended up also updated thefastMNN
function to use an all lowercase prefix (i.e.fastmnn
, notfastMNN
) when saving the dimension reductions into the object. This ensures consistency among all functions which check arguments by converting to lowercase.The
iLISI
function will return a tibble with four columns, where each row represents the score for a given cell. Columns are:iLISI
score for that cellA couple questions I have for reviewers:
integration_method
, but I could just as easily do something likepc_name
. The provided info would then be something likeharmony_PCA
, and then I could split out the method name to include in the final output. I don't think this makes much of a difference.cell_barcode
column is just the barcode, notbarcode-library
format which is how cells are labeled in the merged and integrated SCE objects. Do we think I should keep thebarcode-library
format for this export too?Edit: I just updated this to pull out the code into a function that checks for a properly-provided integration method, since all upcoming functions which calculate metrics will need something like this!