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

Fix remaining assumptions on 5D dimensions #148

Merged
merged 15 commits into from
Jan 19, 2022
Merged

Fix remaining assumptions on 5D dimensions #148

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
5f059c8
Fix remaining assumptions on 5D dimensions
sbesson Dec 10, 2021
a2ba441
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 10, 2021
6165582
Fix mypy errors
sbesson Dec 10, 2021
d61e4d5
Add error at debug level
sbesson Dec 10, 2021
c3927c0
Do not normalize_keys in Format.init_store()
sbesson Dec 10, 2021
d47f25d
Merge remote-tracking branch 'origin/master' into hcs_axes_fix
sbesson Jan 12, 2022
524202d
Fix indexing in PlateLabels spec
sbesson Jan 12, 2022
d02a9c7
Unmark the 2D-5D plate tests as failing
sbesson Jan 12, 2022
81147eb
Add and fix logging statements
will-moore Jan 17, 2022
64cbd11
Don't try to view Plate labels
will-moore Jan 17, 2022
41c0d8f
Update plate reading tests to not expect plate labels
will-moore Jan 17, 2022
f06494e
Merge pull request #1 from will-moore/hcs_axes_fix
sbesson Jan 17, 2022
f1a6199
Merge remote-tracking branch 'origin/master' into hcs_axes_fix
sbesson Jan 18, 2022
55da640
Exclude PlateLabels from codecov
sbesson Jan 18, 2022
962031f
Add comment about PlateLabel.get_pyramid_lazy fix
sbesson Jan 18, 2022
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
24 changes: 16 additions & 8 deletions ome_zarr/reader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,8 @@ def __init__(self, node: Node) -> None:
# Use first Field for rendering settings, shape etc.
image_zarr = self.zarr.create(image_paths[0])
image_node = Node(image_zarr, node)
x_index = len(image_node.metadata["axes"]) - 1
y_index = len(image_node.metadata["axes"]) - 2
level = 0 # load full resolution image
self.numpy_type = image_node.data[level].dtype
self.img_metadata = image_node.metadata
Expand Down Expand Up @@ -439,8 +441,8 @@ def get_lazy_well() -> da.Array:
dtype=self.numpy_type,
)
lazy_row.append(lazy_tile)
lazy_rows.append(da.concatenate(lazy_row, axis=4))
return da.concatenate(lazy_rows, axis=3)
lazy_rows.append(da.concatenate(lazy_row, axis=x_index))
return da.concatenate(lazy_rows, axis=y_index)

node.data = [get_lazy_well()]
node.metadata = image_node.metadata
Expand Down Expand Up @@ -484,8 +486,9 @@ def get_pyramid_lazy(self, node: Node) -> None:

LOGGER.debug("img_pyramid_shapes", well_spec.img_pyramid_shapes)

size_y = well_spec.img_shape[3]
size_x = well_spec.img_shape[4]
self.axes = well_spec.img_metadata["axes"]
size_y = well_spec.img_shape[len(self.axes) - 2]
Copy link
Member

Choose a reason for hiding this comment

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

How close do you think we are getting to wanting accessors for these?

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely feels like something the Axes object proposed in #124 could handle i.e. self.axes.get_y_index() or similar?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thinking more about it, in https://github.com/ome/ome-zarr-py/pull/148/files#diff-b50d9715cc6e4017cfc055fd0ed73ecb5d9158e17f4d58ca5b3ba08b89c46657R581, I used c_index = self.axes.index("c") to avoid relying on magic numbers.

With the current spec, this is also a strategy that could be used here i.e. self.axes.index("y"). However this will also be affected by #124 since the axes representation will become a list of dictionaries rather than a list of strings. @will-moore It looks like we need to decide on a resolution order as this PRs are becoming effectively coupled.

size_x = well_spec.img_shape[len(self.axes) - 1]

# FIXME - if only returning a single stiched plate (not a pyramid)
# need to decide optimal size. E.g. longest side < 1500
Expand Down Expand Up @@ -540,8 +543,9 @@ def get_tile(tile_name: str) -> np.ndarray:

try:
data = self.zarr.load(path)
except ValueError:
except ValueError as e:
LOGGER.error(f"Failed to load {path}")
LOGGER.debug(f"{e}")
data = np.zeros(tile_shape, dtype=self.numpy_type)
return data

Expand All @@ -557,8 +561,8 @@ def get_tile(tile_name: str) -> np.ndarray:
lazy_reader(tile_name), shape=tile_shape, dtype=self.numpy_type
)
lazy_row.append(lazy_tile)
lazy_rows.append(da.concatenate(lazy_row, axis=4))
return da.concatenate(lazy_rows, axis=3)
lazy_rows.append(da.concatenate(lazy_row, axis=len(self.axes) - 1))
return da.concatenate(lazy_rows, axis=len(self.axes) - 2)


class PlateLabels(Plate):
Expand All @@ -573,7 +577,11 @@ def get_tile_path(self, level: int, row: int, col: int) -> str:
def get_pyramid_lazy(self, node: Node) -> None:
super().get_pyramid_lazy(node)
# pyramid data may be multi-channel, but we only have 1 labels channel
node.data[0] = node.data[0][:, 0:1, :, :, :]
if "c" in self.axes:
c_index = self.axes.index("c")
idx = [slice(None)] * len(self.axes)
idx[c_index] = slice(0, 1)
node.data[0] = node.data[0][tuple(idx)]
# remove image metadata
node.metadata = {}

Expand Down
1 change: 0 additions & 1 deletion tests/test_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ def test_multiwells_plate(self, fmt):
for wp in empty_wells:
assert parse_url(str(self.path / wp)) is None

@pytest.mark.xfail(reason="https://github.com/ome/ome-zarr-py/issues/145")
@pytest.mark.parametrize(
"axes, dims",
(
Expand Down