Conversation
Stop downsampling IMAT images and simplify image conversion plumbing. The GET /imat/latest-image endpoint no longer accepts or uses a downsample_factor and now returns full-resolution RGB bytes with shape [height, width, 3]. The image_service.convert_image_to_rgb_array signature was simplified to return (data, width, height) and all resizing logic was removed. Router imports and payload fields related to original/sampled dimensions and downsampleFactor were removed, and tests were updated to match the new full-resolution behavior (including removal of the downsampling unit test).
There was a problem hiding this comment.
Pull request overview
This PR removes downsampling support from the plotting-service IMAT “latest image” flow and updates tests to reflect the simplified response contract.
Changes:
- Simplifies
GET /imat/latest-imageto removedownsample_factorand return onlydataandshape. - Refactors
convert_image_to_rgb_arrayto always return full-resolution RGB data plus dimensions. - Updates plotting-service tests to validate the new helper signature and endpoint payload.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| plotting-service/plotting_service/routers/imat.py | Removes the downsample query param and trims response payload to data + shape. |
| plotting-service/plotting_service/services/image_service.py | Simplifies image conversion helper to full-resolution output and updates return signature. |
| plotting-service/test/test_plotting_api.py | Updates/cleans tests for new helper signature and latest-image response shape. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def testconvert_image_to_rgb_array_returns_data_and_metadata(tmp_path): | ||
| """Ensure images convert to RGB data without altering size when no | ||
| downsampling occurs.""" | ||
| def testconvert_image_to_rgb_array_returns_data_and_shape(tmp_path): |
There was a problem hiding this comment.
Test name testconvert_image_to_rgb_array_returns_data_and_shape is inconsistent with the other tests in this file (which use test_...). Renaming to test_convert_image_to_rgb_array_returns_data_and_shape would improve readability and keep pytest test discovery naming consistent.
| def testconvert_image_to_rgb_array_returns_data_and_shape(tmp_path): | |
| def test_convert_image_to_rgb_array_returns_data_and_shape(tmp_path): |
| def convert_image_to_rgb_array(image_path: Path) -> tuple[list[int], int, int]: | ||
| """Convert image into a RGB byte array to be used by frontend H5Web | ||
| interface. | ||
|
|
||
| :param image_path: Path to the image file | ||
| :param downsample_factor: Factor to reduce resolution (1 keeps original) | ||
| :return: Tuple of (data bytes, original width, original height, sampled width, sampled height) | ||
| :return: Tuple of (data bytes, width, height) | ||
| """ |
There was a problem hiding this comment.
Docstring says "a RGB byte array" and "data bytes", but this function returns a list[int] of byte values. Consider updating wording to "an RGB byte array" and clarifying the return as a list of byte values to match the actual return type.
| # Convert the image to RGB array | ||
| try: | ||
| data, original_width, original_height, sampled_width, sampled_height = convert_image_to_rgb_array( | ||
| latest_path, downsample_factor | ||
| ) | ||
| data, width, height = convert_image_to_rgb_array(latest_path) | ||
| except Exception as exc: | ||
| logger.error("Failed to convert IMAT image at %s", latest_path, exc_info=exc) | ||
| raise HTTPException(HTTPStatus.INTERNAL_SERVER_ERROR, "Unable to convert IMAT image") from exc | ||
|
|
||
| # Calculate effective downsample factor | ||
| effective_downsample = original_width / sampled_width if sampled_width else 1 | ||
|
|
||
| payload = { | ||
| "data": data, | ||
| "shape": [sampled_height, sampled_width, 3], | ||
| "originalWidth": original_width, | ||
| "originalHeight": original_height, | ||
| "sampledWidth": sampled_width, | ||
| "sampledHeight": sampled_height, | ||
| "downsampleFactor": effective_downsample, | ||
| "shape": [height, width, 3], | ||
| } |
There was a problem hiding this comment.
/imat/latest-image now always returns full-resolution pixel data as a JSON list of ints. Compared to the previous default downsample (8x), this can increase response size and server memory/CPU significantly for typical IMAT images; consider adding a hard cap/alternative encoding (e.g., base64 bytes) or explicitly documenting/guarding the maximum supported image dimensions.
Closes #296.
Description
Removes unused downsampling support from the IMAT latest-image flow and aligns the frontend with the simplified API contract.
GET /imat/latest-imageso it no longer acceptsdownsample_factor.originalWidth,originalHeight,sampledWidth,sampledHeight,downsampleFactor) and now return onlydataandshape.convert_image_to_rgb_arrayto always return full-resolution RGB data and dimensions./imat/latest-imagewithoutdownsample_factorand derive sizing fromshape./imat/imagestill uses it during slider interaction.Testing
Passed:
pytest test/test_plotting_api.py -q -k "convert_image_to_rgb_array or latest_imat_image" -p no:cacheprovider