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

Convert scale metadata #118

Merged
merged 7 commits into from
May 3, 2023
Merged

Convert scale metadata #118

merged 7 commits into from
May 3, 2023

Conversation

ziw-liu
Copy link
Collaborator

@ziw-liu ziw-liu commented Apr 20, 2023

Convert pixel and z-step sizes to scale transform in NGFF metadata.

Only implemented for MM OME-TIFF and NDTiff, not for single-page TIFF.

Partial implementation of #103.

only implemented for MM OME-TIFF and NDTiff, not for single-page TIFF

Signed-off-by: Ziwen Liu <67518483+ziw-liu@users.noreply.github.com>
@ziw-liu ziw-liu self-assigned this Apr 20, 2023
@ziw-liu ziw-liu added enhancement New feature or request NGFF OME-NGFF (OME-Zarr format) μManager Micro-Manager files and metadata labels Apr 20, 2023
@ziw-liu
Copy link
Collaborator Author

ziw-liu commented Apr 20, 2023

Convert one of the test datasets:

$ iohub convert -h   
Usage: iohub convert [OPTIONS]

  Converts Micro-Manager TIFF datasets to OME-Zarr

Options:
  -h, --help             Show this message and exit.
  -i, --input DIRECTORY  Input Micro-Manager TIFF dataset directory
                         [required]
  -o, --output PATH      Output zarr store (/**/converted.zarr)  [required]
  -f, --format TEXT      Data type, 'ometiff', 'ndtiff', 'singlepagetiff'
  -g, --grid-layout      Arrange positions in a HCS grid layout
  -p, --label-positions  Dump postion labels in MM metadata to Omero metadata
  -s, --scale-voxels     Write voxel size (XY pixel size and Z-step, in
                         micrometers) as scale coordinate transformation in
                         NGFF

$ iohub convert -i mm2.0-20210713_pm0.13.2_2p_3t_2c_7z_1 -o converted.zarr --scale-voxels
Dataset opened                 
Status: |████████████████|84/84 (Time Remaining: 00:00), 499.05it/s]

Check the converted dataset in napari:

napari-metdata showing modified pixel size

And we get a scale bar in microns!

@ziw-liu ziw-liu marked this pull request as ready for review April 20, 2023 04:28
@ieivanov
Copy link
Contributor

This looks nice! Can scale_voxels be the default behavior of the converter?

@ziw-liu
Copy link
Collaborator Author

ziw-liu commented Apr 20, 2023

This looks nice! Can scale_voxels be the default behavior of the converter?

I made the CLI flag opt-in so that it's default behavior doesn't change. Also it does not work for all datasets so it might be confusing if it fails to find the metadata and just refuse to convert.

But then I actually made it the default on the python API side... This inconsistency should be fixed and I'm not sure which one is better.

@talonchandler
Copy link
Contributor

Agreed, looks very promising!

I would vote with Ivan for scale_voxels being on by default. Maybe --label-positions on by default, too?

Also it does not work for all datasets so it might be confusing if it fails to find the metadata and just refuse to convert.

Maybe throw a warning if the metadata isn't present and continue with the conversion? You could just write ones for the scales if the metadata isn't present.

@ziw-liu
Copy link
Collaborator Author

ziw-liu commented Apr 21, 2023

Maybe throw a warning if the metadata isn't present and continue with the conversion? You could just write ones for the scales if the metadata isn't present.

That's the current implementation.

@ziw-liu
Copy link
Collaborator Author

ziw-liu commented Apr 21, 2023

Also a note about why I'm matching OME-XML with some hand-crafted regex and not using ome-types. The data we have off of MM seems to be non-standard/corrupted/outdated (#66) enough that ome-types throws pydantic ValidationErrors (for wrong types and missing fields) even the XML schema validation is turned off, but also still good enough to extract the metadata we need. This maintenance burden will grow if we keep adding features like this, so just bear in mind...

@ziw-liu
Copy link
Collaborator Author

ziw-liu commented Apr 21, 2023

I would vote with Ivan for scale_voxels being on by default. Maybe --label-positions on by default, too?

A problem is that it is far more intuitive and easier to document/troubleshoot to add feature flags in CLI commands than it is to turn off a default behavior that fails and throws up a scary stack trace. For example having to do cp -r many times is somewhat inconvenient, but having to remember cp-r-as-default --no-recursive is probably worse.

@edyoshikun
Copy link
Contributor

The HCS grid layout is taken from the metadata too right?

@ziw-liu
Copy link
Collaborator Author

ziw-liu commented Apr 21, 2023

The HCS grid layout is taken from the metadata too right?

Not necessarily. By default it will dump everything into a single row. If the -p CLI flag (or label_positions=True in Python) is set, the converter will try to guess a grid of wells from the stage positions. MM high-content screening metadata is not supported.

Copy link
Contributor

@talonchandler talonchandler left a comment

Choose a reason for hiding this comment

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

I tested this branch on an ome-tiff dataset and the conversion failed (same result with and without --scale-voxels). Am I using something incorrectly? Thanks @ziw-liu.

iohub convert -i 2022_08_04_recOrder_pytest_20x_04NA/2T_3P_16Z_128Y_256X_Kazansky_1/ -o ./test.zarr
Traceback (most recent call last):
  File "/opt/anaconda3/envs/iohub/bin/iohub", line 8, in <module>
    sys.exit(cli())
  File "/opt/anaconda3/envs/iohub/lib/python3.9/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/opt/anaconda3/envs/iohub/lib/python3.9/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/opt/anaconda3/envs/iohub/lib/python3.9/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/opt/anaconda3/envs/iohub/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/opt/anaconda3/envs/iohub/lib/python3.9/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/Users/talon.chandler/iohub/iohub/cli/cli.py", line 86, in convert
    converter = TIFFConverter(
  File "/Users/talon.chandler/iohub/iohub/convert.py", line 105, in __init__
    self.reader = read_micromanager(
  File "/Users/talon.chandler/iohub/iohub/reader.py", line 201, in read_micromanager
    return MicromanagerOmeTiffReader(path, extract_data)
  File "/Users/talon.chandler/iohub/iohub/multipagetiff.py", line 53, in __init__
    self._infer_image_meta()
  File "/Users/talon.chandler/iohub/iohub/multipagetiff.py", line 300, in _infer_image_meta
    self.xy_pixel_size = float(xy_size.group())
AttributeError: 'NoneType' object has no attribute 'group'

iohub/cli/cli.py Outdated Show resolved Hide resolved
iohub/convert.py Show resolved Hide resolved
iohub/multipagetiff.py Outdated Show resolved Hide resolved
@ziw-liu
Copy link
Collaborator Author

ziw-liu commented May 2, 2023

Now scaling is the default in CLI.

I tested this branch on an ome-tiff dataset and the conversion failed (same result with and without --scale-voxels). Am I using something incorrectly? Thanks @ziw-liu.

The dataset is missing the metadata field. I couldn't find another field that carries the value, so maybe the microscope was not calibrated to begin with. Now it will convert but log a warning.

@ziw-liu ziw-liu requested a review from talonchandler May 2, 2023 19:52
Copy link
Contributor

@talonchandler talonchandler left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the default behavior so that it doesn't fail when metadata is absent.

I have a suggestion for a better field to transfer from MM...see my comment.

iohub/multipagetiff.py Outdated Show resolved Hide resolved
@ziw-liu ziw-liu merged commit 65ca84f into main May 3, 2023
@ziw-liu ziw-liu deleted the convert-scale-metadata branch May 3, 2023 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request μManager Micro-Manager files and metadata NGFF OME-NGFF (OME-Zarr format)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants