Adding support for Zarr stitching/fusion#41
Adding support for Zarr stitching/fusion#41jwong-nd wants to merge 7 commits intogoogle-research:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
zarr_processor.py
Outdated
| self.tstore = tstore | ||
|
|
||
| def __getitem__(self, ind): | ||
| print(ind) |
zarr_processor.py
Outdated
| class ZarrFusion(warp.StitchAndRender3dTiles): | ||
| """ | ||
| Fusion renderer subclass | ||
| that implements data loading for Zarr datasets. |
There was a problem hiding this comment.
Can probably wrap this into a single line with smth like "Fusion renderer loading tile data from Zarr."
| class CloudStorage(Enum): | ||
| """ | ||
| Documented Cloud Storage Options | ||
| """ |
There was a problem hiding this comment.
Please reduce short docstrings like this one into a single line for consistency with the rest of the code.
zarr_utils.py
Outdated
| downsample_exp: int | ||
|
|
||
|
|
||
| def open_zarr_gcs(bucket: str, path: str): |
There was a problem hiding this comment.
Please add a return type annotation.
mjanusz
left a comment
mjanusz
left a comment
There was a problem hiding this comment.
We're trying to keep the main directory of sofima focused on the main functionality provided by the module. Would you mind moving some files around with this, e.g.
zarr_utils.py -> io/zarr.py
zarr_processor.py -> utils/zarr_register_and_fuse_3d.py
zarr_utils.py
Outdated
| }).result() | ||
|
|
||
|
|
||
| def open_zarr_s3(bucket: str, path: str): |
There was a problem hiding this comment.
Please add a return type annotation.
zarr_utils.py
Outdated
|
|
||
|
|
||
| def load_zarr_data(params: ZarrDataset | ||
| ) -> tuple[list[ts.TensorStore], tuple[int, int, int]]: |
There was a problem hiding this comment.
Consider using stitch_elastic.ShapeXYZ for tuple[int, int, int] which conveys a bit more semantic meaning.
| @dataclass | ||
| class ZarrDataset: | ||
| """ | ||
| Parameters for locating Zarr dataset living on the cloud. |
There was a problem hiding this comment.
Please add an Args: docstring explaining the different parameters (particularly tile_name and downsample_exp).
|
|
||
| # Standardize size of tile volumes | ||
| for i, tile_vol in enumerate(tile_volumes): | ||
| tile_volumes[i] = tile_vol[:, :, :min_z, :min_y, :min_x] |
There was a problem hiding this comment.
Please mention this behavior (tiles getting cropped at origin to the smallest tile in the set) in the docstring.
|
|
||
|
|
||
| def write_zarr(bucket: str, shape: list, path: str): | ||
| """ |
There was a problem hiding this comment.
Please use a single sentence docstring followed by an explanation of the arguments in the Args: section.
| curr_box = bounding_box.BoundingBox(start=(0, 0, 0), size=tile_shape) | ||
| nbor_box = bounding_box.BoundingBox( | ||
| start=( | ||
| tile_shape[0] * (1 - axis) + offset[0], |
There was a problem hiding this comment.
Could you please completely revert the changes to this file?
There was a problem hiding this comment.
Could you please completely revert the changes to this file?
|
Could you please also:
|
2 participants
Hello, this PR adds zarr_processor.py, an entrypoint for processing Zarr datasets.
zarr_processor.py imports:
In addition, I tweaked fine_registration.py and processor.warp.py (deleting unused fields, renaming variables) for readability.