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

Merge method for SFE #29

Open
lambdamoses opened this issue Feb 21, 2024 · 3 comments
Open

Merge method for SFE #29

lambdamoses opened this issue Feb 21, 2024 · 3 comments

Comments

@lambdamoses
Copy link
Collaborator

Kind of like cbind but not quite. In cbind, for all methods that I know of, the number of rows must match. For merge, the rows don't have to perfectly match. The user interface should follow that of base R's merge for data frames whenever applicable, giving options for full join (I think should be default, unlike in base R), left join, right join, and inner join. Give an error when the row names of the two SFE objects have no overlap, and warning when the overlap is small. rowGeometries are subsetted or padded with empty geometries accordingly. rowGeometries of the same name but different content in the two SFE objects are treated the same way as in the SFE cbind method. colGeometries and annotGeometries are simply concatenated as in cbind.

@lambdamoses lambdamoses added enhancement New feature or request intermediate labels Feb 21, 2024
@lambdamoses
Copy link
Collaborator Author

I feel like because the merge method is not specific to the spatial aspect of the data, this is more appropriate as a PR to batchlor (which already has intersectRows, effectively inner_join) or tidySCE to merge SCE objects that don't have the same genes.

@lambdamoses lambdamoses removed enhancement New feature or request intermediate labels Mar 28, 2024
@lambdamoses
Copy link
Collaborator Author

lambdamoses commented Mar 28, 2024

Generally full join filling with 0 is a bad idea, so this is low priority. The correct way is to fill with NA, but this has downstream implications. There already is SummarizedExperiment::combineCols to do a full join. But still cbind and combineCols can get fragile for SFE when there are many colGeometries, since it won't work when some colGeometries have different non-spatial attribute columns. However, I don't expect this scenario to happen very often since I don't often use columns of colGeometries. In any case, it's better to cbind before further analyses. Even without the spatial info, cbinding or merging have implications to data normalization and interpretation of dimension reduction results.

@alikhuseynov
Copy link
Collaborator

I'm just adding here as we discussed.
merge method using inner join, and to deal with mismatches in columns of colGeometry, colData, reducedDims, localResults would via SummarizedExperiment::combineCols.

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

No branches or pull requests

2 participants