Add position labels to ndtiff metadata #166

ieivanov · 2023-07-28T22:40:14Z

This PR adds parsing of position labels for ndtiff datasets and moves hcs_position_labels from MicromanagerOmeTiffReader to ReaderBase as this property now generalizes across multiple readers. This will help us maintain the HCS ome-zarr format for processed mantis datasets.

This PR can be tested with data available in https://github.com/micro-manager/NDTiffStorage/pull/124/files or cm.automaton/mantis_HCS_acq_1

~~Requires new image metadata added in micro-manager/AcqEngJ#95~~

This PR also increments the required ndtiff version from 2.1.0 to 2.2.1

Closes #142

iohub/ndtiff.py

ziw-liu · 2023-07-29T04:28:52Z

Can you clarify the merging timeline for this PR? My reading is that we will wait until the NDTiffStorage and AcqEngJ PRs are merged, and then implement test based on those changes.

ieivanov · 2023-07-31T00:23:01Z

Can you clarify the merging timeline for this PR? My reading is that we will wait until the NDTiffStorage and AcqEngJ PRs are merged, and then implement test based on those changes.

Just tweaked it such that we don't need to rely on metadata added in micro-manager/AcqEngJ#95.

We also don't need to strictly wait on micro-manager/NDStorage#124 to merge, unless we want to reuse that dataset for testing.

Overall, this PR adds new functionality and some safety checks without changing how the reader currently works. We can decide how extensively to test it before we merge. Most tests should focus on the correct function of _check_coordinates and that the axes label are correctly retrieved and parsed in HCS format when present. @ziw-liu should you like to take a lead on adding tests or should I do that? I can help provide the right test datasets

ziw-liu · 2023-07-31T05:30:53Z

iohub/ndtiff.py

@@ -157,7 +228,7 @@ def get_zarr(self, position: int) -> zarr.array:
        # add singleton axes so output is 5D
        return da.reshape(shape)

-    def get_array(self, position: int) -> np.ndarray:
+    def get_array(self, position: Union[int, str]) -> np.ndarray:


The docstring typing needs an update

ziw-liu · 2023-07-31T05:39:59Z

We also don't need to strictly wait on micro-manager/NDTiffStorage#124 to merge, unless we want to reuse that dataset for testing.

I think reusing the upstream dataset would be nice, as long as we don't have to wait too long for it. If you feel that the NDTIFF PR will be merged soon, I can start wrting tests targeting those files as they are and change the URLs when it gets merged.

v2.2.1 adds bug fixes for ndtiff datasets with string-valued axes; we have not used such datasets before, so bumping this version should have no effect on our current workflows

ieivanov · 2023-08-04T20:02:09Z

@ziw-liu micro-manager/NDStorage#124 merged this morning. Can you please add iohub tests for parsing the positions labels in the new datasets in micro-manager/NDStorage#124 and merge this PR?

ziw-liu · 2023-08-04T21:27:23Z

@ieivanov The coordinate check you added does not pass for the test data from NDTiffStorage:

__________________________________________________________ test_v3_labeled_positions __________________________________________________________

ndtiff_v3_labeled_positions = '.pytest_temp/test_data/ndtiff_v3_labeled_positions'

    def test_v3_labeled_positions(ndtiff_v3_labeled_positions):
        data_dir: str = ndtiff_v3_labeled_positions
>       reader = NDTiffReader(data_dir)

tests/reader/test_pycromanager.py:82: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
iohub/ndtiff.py:33: in __init__
    self.mm_meta = self._get_summary_metadata()
iohub/ndtiff.py:43: in _get_summary_metadata
    img_metadata = self.get_image_metadata(0, 0, 0, 0)
iohub/ndtiff.py:269: in get_image_metadata
    p, t, c, z = self._check_coordinates(p, t, c, z)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <iohub.ndtiff.NDTiffReader object at 0x1483ba250>, p = 0, t = 0, c = 0, z = 0

    def _check_coordinates(
        self, p: Union[int, str], t: int, c: Union[int, str], z: int
    ):
        """
        Check that the (p, t, c, z) coordinates are part of the ndtiff dataset.
        Replace coordinates with None or string values in specific cases - see
        below
        """
        coords = [p, t, c, z]
        axes = ("position", "time", "channel", "z")
    
        for i, axis in enumerate(axes):
            coord = coords[i]
    
            # Check if the axis is part of the dataset axes
            if axis in self._axes.keys():
                # Check if coordinate is part of the dataset axis
                if coord in self._axes[axis]:
                    # all good
                    pass
    
                # The requested coordinate is not part of the axis
                else:
                    # If coord=0 is requested and the coordinate axis exists,
                    # but is string valued (e.g. {'Pos0', 'Pos1'}), a warning
                    # will be raised and the coordinate will be replaced by a
                    # random sample.
    
                    # Coordinates are in sets, here we get one sample from the
                    # set without removing it:
                    # https://stackoverflow.com/questions/59825
                    coord_sample = next(iter(self._axes[axis]))
                    if coords == 0 and isinstance(coord_sample, str):
                        coords[i] = coord_sample
                        warnings.warn(
                            f"Indices of {axis} are string-valued. "
                            f"Returning data at {axis} = {coord}"
                        )
                    else:
                        # If the coordinate is not part of the axis and
                        # nonzero, a ValueError will be raised
>                       raise ValueError(
                            f"Image coordinate {axis} = {coord} is not "
                            "part of this dataset."
                        )
E                       ValueError: Image coordinate position = 0 is not part of this dataset.

iohub/ndtiff.py:127: ValueError

ndtiffv3 with labeled positions

ieivanov · 2023-08-04T22:37:57Z

@ziw-liu can you push this test here and then I can troubleshoot?

this test fails due to new axis checking

ieivanov · 2023-08-05T00:05:10Z

@ziw-liu I fixed a bug and modified test_v3_labeled_positions so it checks for the correct position names. This dataset is not intended for parsing HCS labels. We should test that against cm.automaton/mantis_HCS_acq_1

test_modify_hcs_ref still fails, I'm not sure why - can you take a look?

ieivanov · 2023-08-05T00:14:27Z

We should test that these new datasets are correctly converted to zarr, whether they carry HCS or 'Pos0'... position labels

I am guessing that this segment will need to be modified to provide string indices for the position and channel axes, rather than integers. These should be obtained from NDTiffReader._axes, such that you call dataset.as_array(position='Pos1',...) rather than dataset.as_array(position=1,...)

iohub/iohub/convert.py

Lines 254 to 263 in 1ae2737

    
           # create array of coordinate tuples with innermost dimension 
        
           # being the first dim acquired 
        
           self.coords = [ 
        
               (dim3, dim2, dim1, dim0) 
        
               for dim3 in range(dims[3]) 
        
               for dim2 in range(dims[2]) 
        
               for dim1 in range(dims[1]) 
        
               for dim0 in range(dims[0]) 
        
           ]

ziw-liu · 2023-08-05T05:18:31Z

We should test that these new datasets are correctly converted to zarr, whether they carry HCS or 'Pos0'... position labels

I am guessing that this segment will need to be modified to provide string indices for the position and channel axes, rather than integers. These should be obtained from NDTiffReader._axes, such that you call dataset.as_array(position='Pos1',...) rather than dataset.as_array(position=1,...)

iohub/iohub/convert.py

Lines 254 to 263 in 1ae2737

# create array of coordinate tuples with innermost dimension

# being the first dim acquired

self.coords = [

(dim3, dim2, dim1, dim0)

for dim3 in range(dims[3])

for dim2 in range(dims[2])

for dim1 in range(dims[1])

for dim0 in range(dims[0])

]

Do you mean that the converted OME-Zarr will have path names that reflect non-HCS MM stage position labels? Currently we put each position in a separate well in a single row, will this behavior be preserved, thus producing FOV paths 0/Pos000/0, 0/Pos001/0 etc?

ieivanov · 2023-08-05T20:14:52Z

Do you mean that the converted OME-Zarr will have path names that reflect non-HCS MM stage position labels? Currently we put each position in a separate well in a single row, will this behavior be preserved, thus producing FOV paths 0/Pos0/0, 0/Pos1/0 etc?

Yes, I think this will make most sense.

edyoshikun

so far LGTM!

ziw-liu · 2023-08-07T22:47:23Z

The conversion issue will be tracked in #169.

ieivanov added 3 commits July 28, 2023 15:11

add position labels to ndtiff metadata

9a0cf19

move hcs_position_labels to base reader

f65d41d

style

23cce34

ieivanov requested review from talonchandler and ziw-liu July 28, 2023 22:40

ieivanov marked this pull request as draft July 28, 2023 22:57

ieivanov added 4 commits July 28, 2023 16:01

switch to getting position label from PositionName metadata key

34ef96c

more robust ndtiff axis parsing

232f700

support indexing into string axes

d48bbc6

style

4790215

ieivanov marked this pull request as ready for review July 29, 2023 00:51

remove unneeded variable

a1e7d99

ieivanov mentioned this pull request Jul 29, 2023

Save HCS position label in NDTiff dataset axes czbiohub-sf/shrimPy#70

Merged

ziw-liu reviewed Jul 29, 2023

View reviewed changes

iohub/ndtiff.py Outdated Show resolved Hide resolved

ieivanov added 3 commits July 30, 2023 16:32

tweaks

bef1357

Update ndtiff.py

4232150

typing

3be71b2

ziw-liu reviewed Jul 31, 2023

View reviewed changes

ziw-liu added enhancement New feature or request μManager Micro-Manager files and metadata labels Jul 31, 2023

bump ndtiff version

18c2d2e

v2.2.1 adds bug fixes for ndtiff datasets with string-valued axes; we have not used such datasets before, so bumping this version should have no effect on our current workflows

hid pytest_temp

ecc6082

ziw-liu added 2 commits August 4, 2023 14:32

ignore new test data dir

8d5d3df

download dataset from ndtiff github repo

5eb5b23

ndtiffv3 with labeled positions

ziw-liu and others added 3 commits August 4, 2023 15:41

wip: test position label

2990a6e

this test fails due to new axis checking

bug fix

02983b7

update test_v3_labeled_positions

1409025

edyoshikun approved these changes Aug 7, 2023

View reviewed changes

ziw-liu mentioned this pull request Aug 7, 2023

Convert NDTiff with non-HCS position labels #169

Closed

ziw-liu merged commit a4ed7b5 into main Aug 8, 2023

ziw-liu deleted the feature/ndtiff_hcs_positions branch August 8, 2023 00:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add position labels to ndtiff metadata #166

Add position labels to ndtiff metadata #166

ieivanov commented Jul 28, 2023 •

edited

Loading

ziw-liu commented Jul 29, 2023

ieivanov commented Jul 31, 2023

ziw-liu Jul 31, 2023

ziw-liu commented Jul 31, 2023

ieivanov commented Aug 4, 2023

ziw-liu commented Aug 4, 2023

ieivanov commented Aug 4, 2023

ieivanov commented Aug 5, 2023 •

edited

Loading

ieivanov commented Aug 5, 2023 •

edited

Loading

ziw-liu commented Aug 5, 2023

ieivanov commented Aug 5, 2023

edyoshikun left a comment •

edited

Loading

ziw-liu commented Aug 7, 2023

Add position labels to ndtiff metadata #166

Add position labels to ndtiff metadata #166

Conversation

ieivanov commented Jul 28, 2023 • edited Loading

ziw-liu commented Jul 29, 2023

ieivanov commented Jul 31, 2023

ziw-liu Jul 31, 2023

Choose a reason for hiding this comment

ziw-liu commented Jul 31, 2023

ieivanov commented Aug 4, 2023

ziw-liu commented Aug 4, 2023

ieivanov commented Aug 4, 2023

ieivanov commented Aug 5, 2023 • edited Loading

ieivanov commented Aug 5, 2023 • edited Loading

ziw-liu commented Aug 5, 2023

ieivanov commented Aug 5, 2023

edyoshikun left a comment • edited Loading

Choose a reason for hiding this comment

ziw-liu commented Aug 7, 2023

ieivanov commented Jul 28, 2023 •

edited

Loading

ieivanov commented Aug 5, 2023 •

edited

Loading

ieivanov commented Aug 5, 2023 •

edited

Loading

edyoshikun left a comment •

edited

Loading