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.

Sign up for GitHub

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

Jump to bottom

Improve open ijtiff #677

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve open ijtiff #677

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2ed9a1f
Improves `save_ij_tiff` by fixing the issue #671
bugraoezdemir May 6, 2024
fab78b7
Update __init__.py
bugraoezdemir May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions functions/python_modules/OpenIJTIFF/OpenIJTIFF/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import tifffile



def _fetch_data(uri: str) -> Path:
"""Downloads a given file from a uri to a temporary directory

Expand Down Expand Up @@ -173,9 +174,9 @@ def open_ij_tiff(
def save_ij_tiff(
tiffdest: Union[str, Path],
image_array: np.ndarray,
ax_names: Sequence[str],
ax_scales: Sequence[float],
ax_units: Sequence[str],
ax_names: Optional[Sequence[str]] = None,
ax_scales: Optional[Sequence[float]] = None,
ax_units: Optional[Sequence[str]] = None,
Comment on lines +177 to +179
Copy link
Collaborator

Choose a reason for hiding this comment

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

The docstring could mention what happens if these are not supplied. The code suggests that this function assumes a particular axis order. E.g. when a user adds a 3-channel image, they should add a singleton for Z if they are not giving all metadata.

extra_metadata: Optional[dict] = None,
) -> Dict[str, Any]:
"""
Expand All @@ -195,15 +196,33 @@ def save_ij_tiff(
Metadata dictionary
"""

_full_axes = 'TCZYX'
_full_units = ['Frame', 'na', 'Slice', 'Pixel', 'Pixel']

if ax_names is None:
ax_names = _full_axes[-image_array.ndim:]
if ax_units is None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

an improvement here would be to go by axis names already.

axis_units = {k: v for k, v in zip(_full_axes, _full_units)}
ax_units = [axis_units[ax] for ax in ax_names]

this would allow users to specify the axis tags only and still get sensible units

ax_units = _full_units[-image_array.ndim:]
if ax_scales is None:
ax_scales = [1] * image_array.ndim
ax_names = ax_names.upper()
assert len(ax_names) == image_array.ndim, f"The length of 'ax_names' must match the number of dimensions, which is {image_array.ndim}"
assert len(ax_units) == image_array.ndim, f"The length of 'ax_units' must match the number of dimensions, which is {image_array.ndim}"
assert len(ax_scales) == image_array.ndim, f"The length of 'ax_scales' must match the number of dimensions, which is {image_array.ndim}"
Comment on lines +209 to +211
Copy link
Collaborator

Choose a reason for hiding this comment

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

asserts can be removed at runtime if python is started with optimization flags.

try it 
python -c "assert False; print('hehe')"
# will result in AssertionError

python -O -c "assert False; print('hehe')"
# will print

It is safer to do check in an if clause and raise ValueError (or something even more fitting) in case condition is met. After all this error should communicate to the user what went wrong. Assertions are still fine in code in general - I would personally not use them for something that is user input.


metadata = {
"axes": ax_names,
}

if image_array.dtype == np.dtype(bool):
image_array = image_array.astype(np.uint8) * 255
Comment on lines +217 to +218
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also this behavior should be documented in the function docstring.


if "Y" in ax_names:
y_idx = ax_names.index("Y")
metadata["unit"] = ax_units[y_idx]
if "X" in ax_names:
x_idx = ax_names.index("X")
metadata["unit"] = encode_unicode(ax_units[x_idx])
metadata["unit"] = ax_units[x_idx]
if "Z" in ax_names:
z_idx = ax_names.index("Z")
if ax_units[z_idx] != "Slice": ### TODO: This checkpoint might be improved
Expand Down Expand Up @@ -238,3 +257,7 @@ def save_ij_tiff(
metadata=metadata,
)
return metadata




Loading