-
Notifications
You must be signed in to change notification settings - Fork 1
DEGA-290-custom-segmentation-landscape-files-fix #123
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
DEGA-290-custom-segmentation-landscape-files-fix #123
Conversation
…y for cells, qc: trx_matrix addition
cornhundred
left a comment
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. I just asked for a bit of clarification. Can you also merge in the changes from main?
It would be great to turn this notebook, Xenium_V1_human_Pancreas_FFPE_outs.zip, into an automated test but the file sizes are too large.
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.
Thanks for the changes, I just requested a small change to one of the files. You can add some unit tests if you think it makes sense otherwise you can merge.
|
Great work! I will look in more detail later, but I have successfully verified the three notebooks with some adjustments. The I believe this file is generated in pre and saved in the landscape folder and not the raw data folder. If you need it to qc raw data without the need to pre-process. You can simply add this line to your notebook:
|
|
Another required file is missing: This is the content in the json: I suggest something like this: def |
|
Underscore this helper function or remove the function and just implement it in the code as a one-liner. celldega/src/celldega/qc/__init__.py Line 15 in 8dcdcbb
|
|
@jaspreetishar All three notebooks worked on my end. One final question: is this |
huanlity
left a comment
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.
LGTM ✅
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 implements fixes across pre-processing and quality control modules to address custom segmentation landscape files issues. The changes ensure proper handling of cell geometries in micron space and correct function call ordering.
- Fixed function call ordering in
add_custom_segmentationto ensure proper gene metadata handling - Updated cell geometry handling to retain micron space coordinates for accurate QC calculations
- Improved parameter handling and file path resolution across QC and pre-processing modules
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/celldega/qc/init.py | Enhanced QC module with better parameter handling, geometry processing, and data visualization improvements |
| src/celldega/pre/run_pre_processing.py | Added transformation matrix copying for MERSCOPE and fixed cluster creation logic |
| src/celldega/pre/boundary_tile.py | Updated to retain cell geometries in micron space and improved path handling |
| src/celldega/pre/init.py | Fixed function call ordering in custom segmentation and improved DZI file handling |
This PR includes a few minor changes across the pre and qc modules:
The changes have been tested by successfully running the following notebooks: