-
Notifications
You must be signed in to change notification settings - Fork 41
Fix the bugs for paste align: add paste_align_preprocess back #343
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
base: main
Are you sure you want to change the base?
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.
Pull Request Overview
This PR fixes bugs in the paste alignment functionality by restoring the paste_align_preprocess function and correcting API compatibility issues. The changes ensure that paste_align works correctly without crashing due to incorrect function usage.
- Restores the
paste_align_preprocessfunction that was previously removed - Adds a default value for
return_indexparameter in theNumpyBackend.unique()method - Fixes API compatibility by using the correct
cgfunction call
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| spateo/alignment/methods/paste.py | Updates function calls to use paste_align_preprocess and correct cg function |
| spateo/alignment/methods/deprecated_utils.py | Adds the restored paste_align_preprocess function implementation |
| spateo/alignment/methods/backend.py | Adds default value for return_index parameter in unique() method |
| spateo/alignment/methods/init.py | Exports the restored paste_align_preprocess function |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| nx, type_as = check_backend(device=device, dtype=dtype) | ||
| # Subset for common genes | ||
| new_samples = [s.copy() for s in samples] | ||
| all_samples_genes = [s[0].var.index for s in new_samples] |
Copilot
AI
Sep 30, 2025
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.
The indexing s[0] appears incorrect. Each element in new_samples should be an AnnData object, so it should be s.var.index instead of s[0].var.index.
| all_samples_genes = [s[0].var.index for s in new_samples] | |
| all_samples_genes = [s.var.index for s in new_samples] |
| spatial_coords, normalize_scale_list, normalize_mean_list = normalize_coords( | ||
| coords=spatial_coords, nx=nx, verbose=verbose | ||
| ) | ||
| if normalize_g and ((use_rep is None) or (not isinstance(use_rep, str)) or (use_rep not in samples[0].obsm.keys()) or (use_rep not in samples[1].obsm.keys())): |
Copilot
AI
Sep 30, 2025
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 complex condition is duplicated from line 779 and should be extracted into a helper function or variable to improve maintainability and reduce code duplication.
| coords=spatial_coords, nx=nx, verbose=verbose | ||
| ) | ||
| if normalize_g and ((use_rep is None) or (not isinstance(use_rep, str)) or (use_rep not in samples[0].obsm.keys()) or (use_rep not in samples[1].obsm.keys())): | ||
| exp_matrices = normalize_exps(matrices=exp_matrices, nx=nx, verbose=verbose) |
Copilot
AI
Sep 30, 2025
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.
The normalize_exps function expects exp_layers parameter but matrices is being passed. Based on the function signature, this should be exp_layers=exp_matrices.
| exp_matrices = normalize_exps(matrices=exp_matrices, nx=nx, verbose=verbose) | |
| exp_matrices = normalize_exps(exp_layers=exp_matrices, nx=nx, verbose=verbose) |
Fix bugs in paste alignment
Motivation
Fixes an issue where
paste_alignwould crash due to the incorrect use of thealign_preprocessfunction (Closes #341).What’s Changed
paste_align_preprocessfunction, which had previously been removed when introducing the newalign_preprocess.return_indexin theNumpyBackend.unique()function.paste_alignfailed to call the correctcgfunction because of API changes in different versions of the POT package.Validation
st.align.paste_align.Impact
Backward compatible, no API changes.