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

Mcmicro update #28

Merged
merged 22 commits into from
Jan 6, 2024
Merged

Mcmicro update #28

merged 22 commits into from
Jan 6, 2024

Conversation

melonora
Copy link
Collaborator

@melonora melonora commented Apr 2, 2023

This PR updates the code related to creating a SpatialData object out of MCMICRO output. In particular with this PR, tissue micro array output of MCMICRO is supported in addition to the WSI output. Also some code fixes for exemplar_001 are included

One bug has been fixed when passing on image_models_kwargs. When passing dims, this led to problems as the same kwargs were used for the label model. While the image model for MCMICRO should have dims ('c', 'y', 'x') the label model should have dims ('y', 'x').

For tma the config file is read to check whether the data refers to whole slide imaging or tissue micro array.

Currently, for transformations only global with an Identity transform is possible. This can change later. Furthermore, illumination images and dearray masks are now included in the SpatialData object.

Code has been tested on exemplar-001 and exemplar-002 output of MCMICRO.

@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2023

Codecov Report

Attention: 122 lines in your changes are missing coverage. Please review.

Comparison is base (755d475) 41.92% compared to head (7e91a66) 37.56%.
Report is 49 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #28      +/-   ##
==========================================
- Coverage   41.92%   37.56%   -4.36%     
==========================================
  Files          16       14       -2     
  Lines         854     1001     +147     
==========================================
+ Hits          358      376      +18     
- Misses        496      625     +129     
Files Coverage Δ
src/spatialdata_io/_constants/_constants.py 100.00% <100.00%> (ø)
src/spatialdata_io/readers/mcmicro.py 16.33% <6.87%> (-28.11%) ⬇️

... and 6 files with indirect coverage changes

@LucaMarconato
Copy link
Member

LucaMarconato commented Apr 2, 2023

Unfortunately I am not able to test the code at the moment. I can't run the pipeline on exemplar-002 because of a incompatibility with M1 chips labsyspharm/mcmicro#353.

I tried running mc-micro on a linux machine I have remote access to but I got some different errors, and since I only have rootless access, I could not fix them in a straightforward way.

I have added instructions on how to run MCMICRO on the exemplar-001 and exemplar-002 datasets here https://github.com/giovp/spatialdata-sandbox/tree/main/mcmicro_exemplar_io, so someone else can try this. Otherwise I will try again after the paper is out, when I will give more focus on the io scripts.

@LucaMarconato
Copy link
Member

Anyway, from the code points of view, the changes seems great to me.

@melonora
Copy link
Collaborator Author

melonora commented Apr 4, 2023

For trying to get around the problems with the M1 chip, @FloWuenne mentioned that having this in your .bashrc might help:
export DOCKER_DEFAULT_PLATFORM=linux/amd64

@LucaMarconato
Copy link
Member

Thanks @melonora for the work! This together with the Docker support from Flo it's amazing! I will test it on my machine, hopefully this time I will not have M1 related problems in running MCMICRO 🤞

@melonora
Copy link
Collaborator Author

Thanks, can you also particularly check visualization using the view and scatterwidget in napari spatialdata please?

I will pick up the CLI support as well.

@FloWuenne
Copy link

If you run into problems with M1 @LucaMarconato , try out Gitpod for any python features that don't require visualization (so anything outside of napari). Gitpod runs on Ubuntu, so should not face any M1 issues! I will open a PR for a Github action for the docker container and then adjust the Gitpod PR to make it point toward that container!

@LucaMarconato
Copy link
Member

@melonora @FloWuenne I just tried installing nextflow in my M1 machine and it seems not be compatible with the latest Java M1 version (I didn't try with the Java Intel version because I prefer to install Apple Silicon native tools). I reached out to Wouter via chat, I think a fast approach would be if you could upload the output data somewhere in GDrive/S3, or pass it to me with an hard drive.

table[McmicroKeys.INSTANCE_KEY] = "core_" + sample_id + "_" + table[McmicroKeys.INSTANCE_KEY].astype(str)
table.index = table[McmicroKeys.INSTANCE_KEY]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this because the index of the table is never used and this could confuse the user.

@@ -93,37 +94,47 @@ def mcmicro(
marker_names,
)

# in exemplar-001 the raw images are aligned with the illumination images, not with the registration image
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the illumination images (in the case of exemplar-001 we have 6 of them, 2 for each cycle), are not aligned with the registration image. They are instead with the raw images. Each there are 3 cycles (=3 raw images), so I create 3 additional coordinate systems.

@LucaMarconato
Copy link
Member

Thanks Wouter for the great work! I have tested this today on both the exemplar 001 and 002 data and added some changes to fix some transformations and group together the illumination files for a cleaner interface.

Since it has been open for quite a while I am going to merge it now. I have anyway added comments for the fixes that I made directly in the code, please check them out 😊

Finally, here is a GIF showing napari for exemplar-002 (CC @FloWuenne).
CleanShot 2024-01-06 at 16 29 53

LABELS_PREFIX = "unmicst-"
ILLUMINATION_DIR = "illumination"
PARAMS_FILE = "qc/params.yml"
RAW_DIR = "raw"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added this.


# metadata
COORDS_X = "X_centroid"
COORDS_Y = "Y_centroid"
INSTANCE_KEY = "CellID"
ILLUMINATION_SUFFIX_DFP = "-dfp"
ILLUMINATION_SUFFIX_FFP = "-ffp"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed this to recover the name of the coordinate system of the raw images given the filename of illumination files (which ends in -dfp or -ffp.


from spatialdata_io._constants._constants import McmicroKeys

__all__ = ["mcmicro"]


def _get_transformation(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is to align the TMAs to the global image.


return SpatialData(images=images, labels=labels, table=table)


def _get_images(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this because I needed to know the resolutions of the TMA images in order to compute the transformations to the global view. Also I wanted better control on squeeze and on the dims parameter.

@LucaMarconato LucaMarconato merged commit fa07ba3 into scverse:main Jan 6, 2024
6 checks passed
@FloWuenne
Copy link

Wonderful news this got merged! We will add this as an output option to MCMICRO in nf-core soon I am sure 🤩!

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.

mcmicro_io.zip cell and nucleus bitmasks are the same
4 participants