Support single z-stack tif file for input

[matham]

This is part of this PR: https://github.com/brainglobe/cellfinder/pull/397 and https://github.com/brainglobe/brainglobe-workflows/pull/88.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 76.92308% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 90.56%. Comparing base (17e5ebc) to head (befb748). Report is 4 commits behind head on main.

:exclamation: Current head befb748 differs from pull request most recent head ed40e38. Consider uploading reports for the commit ed40e38 to get more accurate results

Files	Patch %	Lines
brainglobe_utils/image_io/load.py	75.00%	3 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #67 +/- ## ========================================== + Coverage 89.07% 90.56% +1.49% ========================================== Files 35 35 Lines 1355 1389 +34 ========================================== + Hits 1207 1258 +51 + Misses 148 131 -17 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[matham]

I added a test as requested.

I had to modify the to_tiff function because by default it seems to set the axis to "qyx" and so the test had the error:

ValueError: Attempted to load ...\pytest-12\test_image_size_tiff_stack0\image_size_tiff_stack.tif but didn't find a xyz-stack. Found qyx axes with shape (4, 4, 4).

My tiff files come via BigStitcher via Fiji, and so have the correct axes set. I'm not sure if that's common. If not and typical imaging tiff files don't have the right axes set, we may have to use the orientation input parameter. Which perhaps we should be relying on exclusively instead of testing the axes names like I did?

Also perhaps get_size_image_from_file_paths is not the function brainreg should call to get the image metadata (the name is certainly not correct anymore as it could be one path 😅 ) and instead there should be corresponding load_xxx_metadata and a load_any_metadata.

[adamltyson]

My tiff files come via BigStitcher via Fiji, and so have the correct axes set. I'm not sure if that's common. If not and typical imaging tiff files don't have the right axes set, we may have to use the orientation input parameter. Which perhaps we should be relying on exclusively instead of testing the axes names like I did?

I think in general it's rare that this type of data comes with correct metadata. There's a lot of homebuilt systems that output files without any metadata.

[K-Meech]

I'd agree that the metadata often isn't set correctly in tiff files. The issue here is it needs to return the image shape in the style: {"x": x_shape, "y": y_shape, "z": z_shape}, so it needs some way to determine the input axis order. I'm not familiar with the orientation parameter - maybe @alessandrofelder has suggestions?

[matham]

I'd agree that the metadata often isn't set correctly in tiff files.

Sounds like I should remove the check then, and not check the metadata for the order of axes.

The issue here is it needs to return the image shape in the style: {"x": x_shape, "y": y_shape, "z": z_shape}, so it needs some way to determine the input axis order. I'm not familiar with the orientation parameter

I meant this parameter that we provide to brainmapper: https://brainglobe.info/documentation/setting-up/image-definition.html.

However, if we're calling directly into cellfinder core through python, this parameter is not present (assuming in the future we'll use these functions to load cellfinder data - moving cellfinder loading functions into this repo)!? Should the load_xxx functions take an input orientation parameter and re-order its output to match the order cellfinder internally expects?

Cell finder internally expects a certain axis order, but it isn't documented anywhere I think. In fact it took me a while to find this and it's still not quite clear to me why we do this: https://github.com/brainglobe/cellfinder/blob/e6a887f1721b89d51328214b108e0da5401a24a9/cellfinder/core/detect/filters/plane/plane_filter.py#L63. I do know when I tried removing the transpose, the subsequent code in cell splitting failed.

[adamltyson]

@alessandrofelder do you know why cellfinder seems to need a specific orientation? The orientation (particularly in plane) shouldn't matter at all. The orientation parameters should only be used for atlas registration.

[adamltyson]

Does anybody know what we need to get this PR merged? I'm slightly confused about the axis thing. As far as I know, we don't use image metadata anywhere in BrainGlobe. In particular, we don't read x/y/z - they are pretty meaningless because everyone has a different idea about what they mean.

[alessandrofelder]

I agree with @adamltyson 's assertions that AFAIK

• we never read image metadata
• orientation shouldn't matter in cellfinder.

I do know when I tried removing the transpose, the subsequent code in cell splitting failed.

Be interesting to know how this failed 🤔 I don't see why removing the transpose would make any difference 😬

[alessandrofelder]

Does anybody know what we need to get this PR merged?

I'd suggest making the transpose question new separate issue, and merging this?

[adamltyson]

I'd suggest making the transpose question new separate issue, and merging this?

Does this affect cellfinder though?

[alessandrofelder]

Does this affect cellfinder though?

Not sure I understand what you mean 🤔 We should reproduce and understand why not transposing the plane in the plane filter seems to break cellfinder (I currently don't), but that's independent of this PR, so it should be a different issue?

[adamltyson]

I just meant does this PR have any impact upon cellfinder? I.e. is there a possibility that merging (and releasing) this, causes cellfinder problems for anyone. Just a naive question based on the comments above.

If not, lets get it merged.

[alessandrofelder]

OK, I follow now, thanks, and good point. We should weaken the error raising code that was added in the PR to warning, or logging before releasing this, to avoid people with bad metadata in their images getting stuck.

[alessandrofelder]

@K-Meech @adamltyson in cases where we have a single tiff file containing a stack with not correctly set metadata, should we

1. assume axis order x-y-z (Cartesian), or
2. assume axis order z-y-x (numpy)

?

[adamltyson]

Do we need to set anything? We don't use the metadata (we define it elsewhere).

[alessandrofelder]

The function get_size_image_from_file_paths returns a dict with keys "x", "y", and "z".

IIUC when we read a 3D tiff we can get the shape of the stack through tiff.series[0].shape which will return a 3-tuple.

If there is metadata that contains xyz in any order, we can use that information to decide which shape dimension we assign to each key for the dict we return.

If there is no axis metadata, or the axis metadata is not xyz in any order, we have to make a decision only on the shape. My question is what that decision should be?

[adamltyson]

Based on usage here and here I think we assume zyx as the default.

As an aside, I'm not sure what we should do with axis in BrainGlobe. It's currently not causing any (major) issues, but we have hundreds of references to axis order and they're not consistent.

I think we should go full numpy and just have axis_0, axis_1 and axis_2. Occasionally if needed we could have e.g. depth to illustrate some specific aspect of the data. However converting everything across 20 repos is going to take ages, for little gain.

Perhaps we should decide a convention, and aim to gradually adopt it?

[alessandrofelder]

I will assume z,y,x ordering for the shape in that case, consistent with Fiji opening an example tif stack and the values for shape returned by tifffile:

[alessandrofelder]

yep, let's gradually aim to go full numpy, with axis_0 as the "depth" and as the number of 2D tiff files in a folder (when we load that kind of data).

[alessandrofelder]

Merging as AFAICT @K-Meech 's requested changes have been addressed.

[matham]

If there's a standard order, It should probably be documented prominently in the code/docs that this is the order used internally!? Because it took me a bit to work out even the order that the input to detection expects. E.g. the docs in detect.py, for main, simply says:

signal_array : numpy.ndarray
        3D array representing the signal data.

But that is probably the first entry point for users that pass data, and from that it's not clear the expected order.

It also wasn't clear in the docs how this all interacts with the brainmapper orientation parameter. Maybe I should open an issue for this!?

[adamltyson]

If there's a standard order, It should probably be documented prominently in the code/docs that this is the order used internally!?

There is, but only sort of. The way that cellfinder loads the data plane-by-plane, there is an assumption that one axis is lower resolution and slower to load. This however isn't necessary. It should probably be documented though.

It also wasn't clear in the docs how this all interacts with the brainmapper orientation parameter. Maybe I should open an issue for this!?

It doesn't interact at all, only in use via brainmapper. I'm not sure we should confuse the docs by adding in something that's unrelated?