From 914e806563900b4135ce0c418b4a7d81590b9530 Mon Sep 17 00:00:00 2001 From: Patrick Austin Date: Fri, 24 Jan 2025 17:35:40 +0000 Subject: [PATCH 1/9] Handle epac_ops_data_version 1.1: parse bit_depth and shift image bits --- operationsgateway_api/src/models.py | 2 + .../src/records/export_handler.py | 26 ++--- .../src/records/false_colour_handler.py | 62 +++++++----- operationsgateway_api/src/records/image.py | 51 ++++++++-- .../src/records/ingestion/channel_checks.py | 8 ++ .../src/records/ingestion/file_checks.py | 4 +- .../src/records/ingestion/hdf_handler.py | 13 +-- operationsgateway_api/src/records/record.py | 99 +++++++++++++++---- operationsgateway_api/src/routes/images.py | 86 ++++++++++------ operationsgateway_api/src/routes/records.py | 14 +-- operationsgateway_api/src/routes/waveforms.py | 13 +-- test/images/test_false_colour_handler.py | 12 +-- test/images/test_false_colour_image_errors.py | 11 --- test/images/test_image.py | 42 ++++---- test/records/ingestion/test_file.py | 2 +- test/records/test_record.py | 4 +- 16 files changed, 292 insertions(+), 157 deletions(-) diff --git a/operationsgateway_api/src/models.py b/operationsgateway_api/src/models.py index 1094cdf9..ba618f79 100644 --- a/operationsgateway_api/src/models.py +++ b/operationsgateway_api/src/models.py @@ -45,6 +45,7 @@ def validate_from_str(input_value: str) -> ObjectId: class ImageModel(BaseModel): path: Optional[Union[str, Any]] data: Optional[Union[np.ndarray, Any]] + bit_depth: Optional[Union[int, Any]] = None model_config = ConfigDict(arbitrary_types_allowed=True) @@ -74,6 +75,7 @@ class ImageChannelMetadataModel(BaseModel): x_pixel_units: Optional[Union[str, Any]] = None y_pixel_size: Optional[Union[float, Any]] = None y_pixel_units: Optional[Union[str, Any]] = None + bit_depth: Optional[Union[int, Any]] = None class ImageChannelModel(BaseModel): diff --git a/operationsgateway_api/src/records/export_handler.py b/operationsgateway_api/src/records/export_handler.py index 4097920c..1394f7d9 100644 --- a/operationsgateway_api/src/records/export_handler.py +++ b/operationsgateway_api/src/records/export_handler.py @@ -92,12 +92,13 @@ async def process_records(self) -> None: if self.functions: await Record.apply_functions( - record_data, - self.functions, - self.original_image, - self.lower_level, - self.upper_level, - self.colourmap_name, + record=record_data, + functions=self.functions, + original_image=self.original_image, + lower_level=self.lower_level, + upper_level=self.upper_level, + limit_bit_depth=8, # Assume the limit bit depth for the export + colourmap_name=self.colourmap_name, return_thumbnails=False, ) @@ -264,12 +265,13 @@ async def _add_image_to_zip( image_bytes = channel["data"] else: image_bytes = await Image.get_image( - record_id, - channel_name, - self.original_image, - self.lower_level, - self.upper_level, - self.colourmap_name, + record_id=record_id, + channel_name=channel_name, + original_image=self.original_image, + lower_level=self.lower_level, + upper_level=self.upper_level, + limit_bit_depth=8, # Assume the limit bit depth for the export + colourmap_name=self.colourmap_name, ) self.zip_file.writestr( f"{record_id}_{channel_name}.png", diff --git a/operationsgateway_api/src/records/false_colour_handler.py b/operationsgateway_api/src/records/false_colour_handler.py index b9ae9586..52a4f141 100644 --- a/operationsgateway_api/src/records/false_colour_handler.py +++ b/operationsgateway_api/src/records/false_colour_handler.py @@ -45,6 +45,7 @@ async def get_preferred_colourmap( def create_colourbar( lower_level: int, upper_level: int, + limit_bit_depth: int, colourmap_name: str, ) -> BytesIO: """ @@ -60,11 +61,12 @@ def create_colourbar( range(256) for _ in range(FalseColourHandler.colourbar_height_pixels) ] return FalseColourHandler.apply_false_colour( - image_array, - 8, - lower_level, - upper_level, - colourmap_name, + image_array=image_array, + storage_bit_depth=8, + lower_level=lower_level, + upper_level=upper_level, + limit_bit_depth=limit_bit_depth, + colourmap_name=colourmap_name, ) @staticmethod @@ -83,17 +85,19 @@ def apply_false_colour_to_b64_img( return FalseColourHandler.apply_false_colour( image_array, FalseColourHandler.get_pixel_depth(img_src), - lower_level, - upper_level, - colourmap_name, + lower_level=lower_level, + upper_level=upper_level, + limit_bit_depth=8, # All thumbnails are 8 bit, so limits should be too + colourmap_name=colourmap_name, ) @staticmethod def apply_false_colour( image_array: np.ndarray, - bits_per_pixel: int, + storage_bit_depth: int, lower_level: int, upper_level: int, + limit_bit_depth: int, colourmap_name: str, ) -> BytesIO: """ @@ -102,9 +106,10 @@ def apply_false_colour( retrieved as base 64 from the database, or from an image stored on disk. """ vmin, vmax = FalseColourHandler.pixel_limits( - bits_per_pixel, - lower_level, - upper_level, + storage_bit_depth=storage_bit_depth, + lower_level=lower_level, + upper_level=upper_level, + limit_bit_depth=limit_bit_depth, ) if colourmap_name is None: colourmap_name = FalseColourHandler.default_colour_map_name @@ -128,39 +133,46 @@ def apply_false_colour( @staticmethod def pixel_limits( - bits_per_pixel: int, + storage_bit_depth: int, lower_level: int, upper_level: int, + limit_bit_depth: int, ) -> "tuple[int, int]": - """Adjusts pixel limits to account for the number of `bits_per_pixel`. + """Adjusts pixel limits to account for the bit depth the image was actually + saved with. Args: - bits_per_pixel (int): - Bits of depth to each pixel, such that the max value is - `2**bits_per_pixel - 1` - lower_level (int): Low pixel value in 8 bit depth - upper_level (int): High pixel value in 8 bit depth + storage_bit_depth (int): + Bit depth of each pixel in the stored format, such that the max value is + `2**actual_bit_depth - 1` + lower_level (int): Low pixel value in `limit_bit_depth` + upper_level (int): High pixel value in `limit_bit_depth` + limit_bit_depth (int): The bit depth used for the limit levels provided Raises: - ImageError: If `bits_per_pixel` is neither `8` nor `16` - QueryParameterError: If `lower_level` is greater than `upper_level` + QueryParameterError: + If `lower_level` is greater than `upper_level` or `upper_level` is + greater than or equal to 2**`limit_bit_depth`. Returns: tuple[int, int]: The scaled limits """ - if bits_per_pixel != 8 and bits_per_pixel != 16: - raise ImageError(f"{bits_per_pixel} bits per pixel is not supported") if lower_level is None: lower_level = 0 + if upper_level is None: - upper_level = 255 + upper_level = 2**limit_bit_depth - 1 + elif upper_level >= 2**limit_bit_depth: + msg = "upper_level must be less than 2**limit_bit_depth" + raise QueryParameterError(msg) + if upper_level < lower_level: raise QueryParameterError( "lower_level must be less than or equal to upperlevel", ) - pixel_multiplier = 2 ** (bits_per_pixel - 8) + pixel_multiplier = 2 ** (storage_bit_depth - limit_bit_depth) vmin = lower_level * pixel_multiplier vmax = (upper_level + 1) * pixel_multiplier - 1 return vmin, vmax diff --git a/operationsgateway_api/src/records/image.py b/operationsgateway_api/src/records/image.py index f4a25c4d..95ac3504 100644 --- a/operationsgateway_api/src/records/image.py +++ b/operationsgateway_api/src/records/image.py @@ -6,7 +6,7 @@ from botocore.exceptions import ClientError import numpy as np -from PIL import Image as PILImage +from PIL import Image as PILImage, PngImagePlugin from pydantic import Json from operationsgateway_api.src.auth.jwt_handler import JwtHandler @@ -37,6 +37,33 @@ class Image: def __init__(self, image: ImageModel) -> None: self.image = image + if isinstance(self.image.bit_depth, int): + bit_depth = self.image.bit_depth + if bit_depth <= 8 and self.image.data.dtype != np.uint8: + msg = ( + "Specified bit depth %s is lower than actual bit depth with dtype " + "of %s, least significant bits will be lost" + ) + log.warning(msg, bit_depth, self.image.data.dtype) + else: + if self.image.data.dtype == np.uint8: + bit_depth = 8 + else: + # In principle this could be data with any number of bits, but we have + # no way of knowing without an explicit bit_depth so default to 16 + bit_depth = 16 + + # We only store data as either 8 or 16 bit PNGs, scale to most significant bits + if bit_depth <= 8: + target_bit_depth = 8 + target_dtype = np.uint8 + else: + target_bit_depth = 16 + target_dtype = np.uint16 + + self.image.data *= 2 ** (target_bit_depth - bit_depth) + self.image.data = self.image.data.astype(target_dtype) + def create_thumbnail(self) -> None: """ Using the object's image data, create a thumbnail of the image and store it as @@ -88,7 +115,13 @@ def upload_image(input_image: Image) -> None: image_bytes = BytesIO() try: image = PILImage.fromarray(input_image.image.data) - image.save(image_bytes, format="PNG") + if input_image.image.bit_depth is not None: + info = PngImagePlugin.PngInfo() + sbit = input_image.image.bit_depth.to_bytes(1, byteorder="big") + info.add(b"sBIT", sbit) + image.save(image_bytes, format="PNG", pnginfo=info) + else: + image.save(image_bytes, format="PNG") except TypeError as exc: log.exception(msg=exc) raise ImageError("Image data is not in correct format to be read") from exc @@ -105,6 +138,7 @@ async def get_image( original_image: bool, lower_level: int, upper_level: int, + limit_bit_depth: int, colourmap_name: str, ) -> BytesIO: """ @@ -142,13 +176,14 @@ async def get_image( ) img_src = PILImage.open(image_bytes) orig_img_array = np.array(img_src) - + storage_bit_depth = FalseColourHandler.get_pixel_depth(img_src) false_colour_image = FalseColourHandler.apply_false_colour( - orig_img_array, - FalseColourHandler.get_pixel_depth(img_src), - lower_level, - upper_level, - colourmap_name, + image_array=orig_img_array, + storage_bit_depth=storage_bit_depth, + lower_level=lower_level, + upper_level=upper_level, + limit_bit_depth=limit_bit_depth, + colourmap_name=colourmap_name, ) img_src.close() return false_colour_image diff --git a/operationsgateway_api/src/records/ingestion/channel_checks.py b/operationsgateway_api/src/records/ingestion/channel_checks.py index ed134992..2b9b7aa4 100644 --- a/operationsgateway_api/src/records/ingestion/channel_checks.py +++ b/operationsgateway_api/src/records/ingestion/channel_checks.py @@ -249,6 +249,14 @@ def image_metadata_checks(cls, key, value_dict, rejected_channels): rejected_channels.append( {key: "y_pixel_units attribute has wrong datatype"}, ) + + if ( + "bit_depth" in value_dict + and value_dict["bit_depth"] is not None + and not isinstance(value_dict["bit_depth"], int) + ): + rejected_channels.append({key: "bit_depth attribute has wrong datatype"}) + return rejected_channels def optional_dtype_checks(self): diff --git a/operationsgateway_api/src/records/ingestion/file_checks.py b/operationsgateway_api/src/records/ingestion/file_checks.py index 8561a860..fcaee7ac 100644 --- a/operationsgateway_api/src/records/ingestion/file_checks.py +++ b/operationsgateway_api/src/records/ingestion/file_checks.py @@ -47,11 +47,11 @@ def epac_data_version_checks(self): raise RejectFileError( "epac_ops_data_version major version was not 1", ) - if int(epac_numbers[1]) > 0: + if int(epac_numbers[1]) > 1: log.warning( "epac_ops_data_version minor version: %s", epac_numbers[1], ) - return "File minor version number too high (expected 0)" + return "File minor version number too high (expected <=1)" else: raise RejectFileError("epac_ops_data_version does not exist") diff --git a/operationsgateway_api/src/records/ingestion/hdf_handler.py b/operationsgateway_api/src/records/ingestion/hdf_handler.py index decef107..c055a514 100644 --- a/operationsgateway_api/src/records/ingestion/hdf_handler.py +++ b/operationsgateway_api/src/records/ingestion/hdf_handler.py @@ -122,14 +122,15 @@ def _extract_image( return None, internal_failed_channel try: - self.images.append( - ImageModel(path=image_path, data=value["data"][()]), + metadata = ImageChannelMetadataModel(**channel_metadata) + channel = ImageChannelModel(metadata=metadata, image_path=image_path) + image_model = ImageModel( + path=image_path, + data=value["data"][()], + bit_depth=metadata.bit_depth, ) + self.images.append(image_model) - channel = ImageChannelModel( - metadata=ImageChannelMetadataModel(**channel_metadata), - image_path=image_path, - ) return channel, False except KeyError: internal_failed_channel.append( diff --git a/operationsgateway_api/src/records/record.py b/operationsgateway_api/src/records/record.py index 41630b27..c6f0e95c 100644 --- a/operationsgateway_api/src/records/record.py +++ b/operationsgateway_api/src/records/record.py @@ -345,6 +345,44 @@ async def get_channel_dtype( return channel_dtype + @staticmethod + async def get_raw_bit_depth( + record_id: str, + channel_name: str, + channel_value: dict, + ) -> "int | None": + """ + Extract "bit_depth" from `channel_value`, or if not present, retrieve with a + separate lookup. + + Args: + record_id (str): Record identifier + channel_name (str): Channel name to get the bit depth for + channel_value (dict): + Previously fetched channel (may not include all the metadata). + + Returns: + int | None: The bit_depth if found, `None` otherwise. + """ + try: + raw_bit_depth = channel_value["metadata"]["bit_depth"] + except KeyError: + # if a projection has been applied then the record will only contain + # the requested fields and probably not the channel_dtype + # so it needs to be looked up separately + record_dict = await Record.find_record_by_id( + record_id, + {}, + [f"channels.{channel_name}.metadata.bit_depth"], + ) + metadata = record_dict["channels"][channel_name]["metadata"] + if "bit_depth" in metadata: + raw_bit_depth = metadata["bit_depth"] + else: + raw_bit_depth = None + + return raw_bit_depth + @staticmethod async def convert_search_ranges(date_range, shotnum_range): if date_range: @@ -422,6 +460,7 @@ async def apply_functions( original_image: bool, lower_level: int, upper_level: int, + limit_bit_depth: int, colourmap_name: str, return_thumbnails: bool = True, truncate: bool = False, @@ -441,6 +480,7 @@ async def apply_functions( original_image=original_image, lower_level=lower_level, upper_level=upper_level, + limit_bit_depth=limit_bit_depth, colourmap_name=colourmap_name, variable_data=variable_data, variable_transformer=variable_transformer, @@ -465,6 +505,7 @@ async def _apply_function( original_image: bool, lower_level: int, upper_level: int, + limit_bit_depth: int, colourmap_name: str, variable_data: dict, variable_transformer: VariableTransformer, @@ -512,6 +553,7 @@ async def _apply_function( original_image=original_image, lower_level=lower_level, upper_level=upper_level, + limit_bit_depth=limit_bit_depth, colourmap_name=colourmap_name, function_name=function["name"], result=result, @@ -576,17 +618,33 @@ async def _extract_variable( channel_value=channel_value, ) if channel_dtype == "image": + raw_bit_depth = await Record.get_raw_bit_depth( + record_id=record_id, + channel_name=name, + channel_value=channel_value, + ) image_bytes = await Image.get_image( - record_id, - name, - True, - 0, - 255, - None, + record_id=record_id, + channel_name=name, + original_image=True, + lower_level=0, + upper_level=255, + limit_bit_depth=8, # Not relevant when `original_image=True` + colourmap_name=None, ) img_src = PILImage.open(image_bytes) img_array = np.array(img_src) - return img_array + if raw_bit_depth in (None, 8, 16): + # If we don't know the original bit depth, or it exactly matches a + # storage depth, no shift is possible/needed + return img_array + elif raw_bit_depth < 8: + # Bit depths < 8 would have been stored as 8 bit, so shift back to raw + return img_array / 2 ** (8 - raw_bit_depth) + else: + # Bit depths > 8 would have been stored as 16 bit, so shift back to raw + return img_array / 2 ** (16 - raw_bit_depth) + elif channel_dtype == "waveform": waveform_path = channel_value["waveform_path"] if "metadata" in channel_value and "x_units" in channel_value["metadata"]: @@ -605,6 +663,7 @@ def _parse_function_results( original_image: bool, lower_level: int, upper_level: int, + limit_bit_depth: int, colourmap_name: str, function_name: str, result: "np.ndarray | WaveformVariable | np.float64", @@ -623,6 +682,7 @@ def _parse_function_results( original_image=original_image, lower_level=lower_level, upper_level=upper_level, + limit_bit_depth=limit_bit_depth, colourmap_name=colourmap_name, return_thumbnails=return_thumbnails, truncate=truncate, @@ -656,6 +716,7 @@ def _parse_image_result( original_image: bool, lower_level: int, upper_level: int, + limit_bit_depth: int, colourmap_name: str, return_thumbnails: bool, truncate: bool, @@ -665,7 +726,7 @@ def _parse_image_result( """ # We do not track the bit depth of inputs, so keep maximum depth to # avoid losing information - bits_per_pixel = 16 + storage_bit_depth = 16 if return_thumbnails: metadata = { "channel_dtype": "image", @@ -683,11 +744,12 @@ def _parse_image_result( # Slice with a step size that downsamples to the thumbnails shape image_array = result[::step_y, ::step_x] image_bytes = FalseColourHandler.apply_false_colour( - image_array, - bits_per_pixel, - lower_level, - upper_level, - colourmap_name, + image_array=image_array, + storage_bit_depth=storage_bit_depth, + lower_level=lower_level, + upper_level=upper_level, + limit_bit_depth=limit_bit_depth, + colourmap_name=colourmap_name, ) image_bytes.seek(0) image_b64 = base64.b64encode(image_bytes.getvalue()) @@ -707,11 +769,12 @@ def _parse_image_result( img_temp.save(image_bytes, format="PNG") else: image_bytes = FalseColourHandler.apply_false_colour( - result, - bits_per_pixel, - lower_level, - upper_level, - colourmap_name, + image_array=result, + storage_bit_depth=storage_bit_depth, + lower_level=lower_level, + upper_level=upper_level, + limit_bit_depth=limit_bit_depth, + colourmap_name=colourmap_name, ) image_bytes.seek(0) diff --git a/operationsgateway_api/src/routes/images.py b/operationsgateway_api/src/routes/images.py index b485c623..dcd88dca 100644 --- a/operationsgateway_api/src/routes/images.py +++ b/operationsgateway_api/src/routes/images.py @@ -58,6 +58,17 @@ async def get_full_image( ge=0, le=255, ), + limit_bit_depth: int = Query( + 8, + description=( + "The bit depth to which `lower_level` and `upper_level` are relative. This " + "can be any value as long as it is consistent with the provided levels, " + "but is intended to reflect the original bit depth of the raw data, so " + "meaningful values can be displayed and set in the frontend." + ), + ge=0, + le=16, + ), original_image: Optional[bool] = Query( False, description="Return the original image in PNG format without any" @@ -82,13 +93,14 @@ async def get_full_image( colourmap_name = await Image.get_preferred_colourmap(access_token) image_bytes = await get_image_bytes( - record_id, - channel_name, - colourmap_name, - upper_level, - lower_level, - original_image, - functions, + record_id=record_id, + channel_name=channel_name, + colourmap_name=colourmap_name, + upper_level=upper_level, + lower_level=lower_level, + limit_bit_depth=limit_bit_depth, + original_image=original_image, + functions=functions, ) # ensure the "file pointer" is reset image_bytes.seek(0) @@ -145,13 +157,14 @@ async def get_crosshair_intensity( evaluated to generate the returned image. """ image_bytes = await get_image_bytes( - record_id, - channel_name, - None, - 255, - 0, - True, - functions, + record_id=record_id, + channel_name=channel_name, + colourmap_name=None, + upper_level=255, + lower_level=0, + limit_bit_depth=8, + original_image=True, + functions=functions, ) img_src = PILImage.open(image_bytes) orig_img_array = np.array(img_src) @@ -190,6 +203,17 @@ async def get_colourbar_image( ge=0, le=255, ), + limit_bit_depth: int = Query( + 8, + description=( + "The bit depth to which `lower_level` and `upper_level` are relative. This " + "can be any value as long as it is consistent with the provided levels, " + "but is intended to reflect the original bit depth of the raw data, so " + "meaningful values can be displayed and set in the frontend." + ), + ge=0, + le=16, + ), ): """ This endpoint can be used to retrieve a colourbar image showing the colour spectrum @@ -202,9 +226,10 @@ async def get_colourbar_image( colourmap_name = await Image.get_preferred_colourmap(access_token) colourbar_image_bytes = FalseColourHandler.create_colourbar( - lower_level, - upper_level, - colourmap_name, + lower_level=lower_level, + upper_level=upper_level, + limit_bit_depth=limit_bit_depth, + colourmap_name=colourmap_name, ) colourbar_image_bytes.seek(0) return Response(colourbar_image_bytes.read(), media_type="image/png") @@ -238,6 +263,7 @@ async def get_image_bytes( colourmap_name: str, upper_level: int, lower_level: int, + limit_bit_depth: int, original_image: bool, functions: "list[dict]", ) -> BytesIO: @@ -249,21 +275,23 @@ async def get_image_bytes( if function_dict["name"] == channel_name: record = {"_id": record_id} await Record.apply_functions( - record, - functions, - original_image, - lower_level, - upper_level, - colourmap_name, + record=record, + functions=functions, + original_image=original_image, + lower_level=lower_level, + upper_level=upper_level, + limit_bit_depth=limit_bit_depth, + colourmap_name=colourmap_name, return_thumbnails=False, ) return record["channels"][channel_name]["data"] return await Image.get_image( - record_id, - channel_name, - original_image, - lower_level, - upper_level, - colourmap_name, + record_id=record_id, + channel_name=channel_name, + original_image=original_image, + lower_level=lower_level, + upper_level=upper_level, + limit_bit_depth=limit_bit_depth, + colourmap_name=colourmap_name, ) diff --git a/operationsgateway_api/src/routes/records.py b/operationsgateway_api/src/routes/records.py index 11e7b941..c17c3f2b 100644 --- a/operationsgateway_api/src/routes/records.py +++ b/operationsgateway_api/src/routes/records.py @@ -117,12 +117,14 @@ async def get_records( if functions: await Record.apply_functions( - record_data, - functions, - False, - lower_level, - upper_level, - colourmap_name, + record=record_data, + functions=functions, + original_image=False, + lower_level=lower_level, + upper_level=upper_level, + limit_bit_depth=8, # We are returning thumbnails, so limits are 8 bit + colourmap_name=colourmap_name, + return_thumbnails=True, truncate=truncate, ) diff --git a/operationsgateway_api/src/routes/waveforms.py b/operationsgateway_api/src/routes/waveforms.py index bdf3c480..7c911f48 100644 --- a/operationsgateway_api/src/routes/waveforms.py +++ b/operationsgateway_api/src/routes/waveforms.py @@ -63,12 +63,13 @@ async def get_waveform_by_id( record = {"_id": record_id} colourmap_name = await Image.get_preferred_colourmap(access_token) await Record.apply_functions( - record, - functions, - False, - 0, - 255, - colourmap_name, + record=record, + functions=functions, + original_image=False, + lower_level=0, + upper_level=255, + limit_bit_depth=8, # Limits hardcoded to 8 bit + colourmap_name=colourmap_name, return_thumbnails=False, ) diff --git a/test/images/test_false_colour_handler.py b/test/images/test_false_colour_handler.py index c33c95d2..fdc545b0 100644 --- a/test/images/test_false_colour_handler.py +++ b/test/images/test_false_colour_handler.py @@ -1,6 +1,5 @@ import pytest -from operationsgateway_api.src.exceptions import ImageError from operationsgateway_api.src.records.false_colour_handler import FalseColourHandler @@ -67,16 +66,7 @@ def test_pixel_limits( bits_per_pixel, lower_level, upper_level, + limit_bit_depth=8, ) assert vmin == vmin_expected[bits_per_pixel] assert vmax == vmax_expected[bits_per_pixel] - - def test_pixel_limits_error(self): - with pytest.raises(ImageError) as e: - FalseColourHandler.pixel_limits(12, None, None) - - message = ( - "operationsgateway_api.src.exceptions.ImageError: " - "12 bits per pixel is not supported" - ) - assert e.exconly() == message diff --git a/test/images/test_false_colour_image_errors.py b/test/images/test_false_colour_image_errors.py index c90994f7..e1b6746f 100644 --- a/test/images/test_false_colour_image_errors.py +++ b/test/images/test_false_colour_image_errors.py @@ -6,17 +6,6 @@ class TestFalseColour: - def test_invalid_bits_per_pixel(self): - - with pytest.raises(ImageError, match="7 bits per pixel is not supported"): - FalseColourHandler.apply_false_colour( - image_array="", - bits_per_pixel="7", - lower_level=4, - upper_level=8, - colourmap_name="test", - ) - def test_invalid_image_mode(self): image_path = "test/images/jet_image.png" pil_image = Image.open(image_path).convert("RGB") diff --git a/test/images/test_image.py b/test/images/test_image.py index dc9d3c20..1daaf37b 100644 --- a/test/images/test_image.py +++ b/test/images/test_image.py @@ -51,7 +51,7 @@ def test_create_thumbnail(self): test_image = Image( ImageModel( path="test/path/photo.png", - data=np.ones(shape=(300, 300), dtype=np.int8), + data=np.ones(shape=(300, 300), dtype=np.uint8), ), ) test_image.create_thumbnail() @@ -59,9 +59,7 @@ def test_create_thumbnail(self): bytes_thumbnail = base64.b64decode(test_image.thumbnail) img = PILImage.open(BytesIO(bytes_thumbnail)) thumbnail_checksum = str(imagehash.phash(img)) - assert thumbnail_checksum == "0000000000000000" - # the reason only 0s are being asserted is because this is checking the hash of - # a purely black 300x300 square created in the test_image above + assert thumbnail_checksum == "8000000000000000" @pytest.mark.parametrize( # image_size parameter = (rows, columns), not (width, height) which is what @@ -102,7 +100,7 @@ def test_create_thumbnail_config_size( test_image = Image( ImageModel( path="test/path/photo.png", - data=np.ones(shape=image_size, dtype=np.int8), + data=np.ones(shape=image_size, dtype=np.uint8), ), ) with patch( @@ -119,7 +117,7 @@ def test_alternative_mode_thumbnail(self): test_image = Image( ImageModel( path="test/path/photo.png", - data=np.ones(shape=(300, 300), dtype=np.int8), + data=np.ones(shape=(300, 300), dtype=np.uint8), ), ) @@ -163,7 +161,7 @@ def test_extract_metadata( expected_channel_name, ): test_image = Image( - ImageModel(path=image_path, data=np.ones(shape=(300, 300), dtype=np.int8)), + ImageModel(path=image_path, data=np.ones(shape=(300, 300), dtype=np.uint8)), ) record_id, channel_name = test_image.extract_metadata_from_path() @@ -195,7 +193,7 @@ def test_valid_upload_image(self, mock_upload_file_object, _): test_image = Image( ImageModel( path=self.test_image_path, - data=np.ones(shape=(300, 300), dtype=np.int8), + data=np.ones(shape=(300, 300), dtype=np.uint8), ), ) Image.upload_image(test_image) @@ -234,7 +232,7 @@ def test_invalid_upload_image(self, _, __): test_image = Image( ImageModel( path=self.test_image_path, - data=np.ones(shape=(300, 300), dtype=np.int8), + data=np.ones(shape=(300, 300), dtype=np.uint8), ), ) with pytest.raises(ImageError): @@ -282,12 +280,13 @@ async def test_valid_get_image( return_value=self._get_bytes_of_image(expected_image_filename), ): test_image = await Image.get_image( - "test_record_id", - "test_channel_name", - original_image_flag, - 0, - 255, - "jet", + record_id="test_record_id", + channel_name="test_channel_name", + original_image=original_image_flag, + lower_level=0, + upper_level=255, + limit_bit_depth=8, + colourmap_name="jet", ) assert ( @@ -334,12 +333,13 @@ async def test_invalid_get_image(self, _, __, expected_exception, record_count): ): with pytest.raises(expected_exception): await Image.get_image( - "test_record_id", - "test_channel_name", - True, - 0, - 255, - "jet", + record_id="test_record_id", + channel_name="test_channel_name", + original_image=True, + lower_level=0, + upper_level=255, + limit_bit_depth=8, + colourmap_name="jet", ) def test_get_relative_path(self): diff --git a/test/records/ingestion/test_file.py b/test/records/ingestion/test_file.py index 34927535..5cb61f2f 100644 --- a/test/records/ingestion/test_file.py +++ b/test/records/ingestion/test_file.py @@ -22,7 +22,7 @@ async def test_minor_version_too_high(self, remove_hdf_file): assert ( file_checker.epac_data_version_checks() - == "File minor version number too high (expected 0)" + == "File minor version number too high (expected <=1)" ) @pytest.mark.parametrize( diff --git a/test/records/test_record.py b/test/records/test_record.py index 93c5a26f..73d45948 100644 --- a/test/records/test_record.py +++ b/test/records/test_record.py @@ -524,6 +524,7 @@ async def test_apply_functions( original_image=False, lower_level=0, upper_level=255, + limit_bit_depth=8, colourmap_name="binary", return_thumbnails=True, ) @@ -568,7 +569,7 @@ async def test_apply_functions_failure( ): record = {"_id": "20230605100000"} with pytest.raises(FunctionParseError) as e: - await Record.apply_functions(record, functions, False, 0, 255, "binary") + await Record.apply_functions(record, functions, False, 0, 255, 8, "binary") assert str(e.value) == "b is not known as a channel or function name" @@ -588,6 +589,7 @@ async def test_apply_functions_missing_channel(self): original_image=False, lower_level=0, upper_level=255, + limit_bit_depth=8, colourmap_name="binary", return_thumbnails=True, ) From 936eee9566f87949543b12348883cd3881796e04 Mon Sep 17 00:00:00 2001 From: Patrick Austin Date: Mon, 27 Jan 2025 14:00:06 +0000 Subject: [PATCH 2/9] Refactor _bit_shift_to_raw and unit tests for bit shifting --- operationsgateway_api/src/records/image.py | 8 +- operationsgateway_api/src/records/record.py | 45 ++++--- test/images/test_false_colour_handler.py | 11 ++ test/images/test_image.py | 129 +++++++++++++++++++- test/records/ingestion/create_test_hdf.py | 2 + test/records/ingestion/test_channel.py | 5 + test/records/test_record.py | 11 ++ 7 files changed, 191 insertions(+), 20 deletions(-) diff --git a/operationsgateway_api/src/records/image.py b/operationsgateway_api/src/records/image.py index 95ac3504..b69b564f 100644 --- a/operationsgateway_api/src/records/image.py +++ b/operationsgateway_api/src/records/image.py @@ -41,10 +41,10 @@ def __init__(self, image: ImageModel) -> None: bit_depth = self.image.bit_depth if bit_depth <= 8 and self.image.data.dtype != np.uint8: msg = ( - "Specified bit depth %s is lower than actual bit depth with dtype " - "of %s, least significant bits will be lost" + "Specified bit depth is lower than actual bit depth with dtype " + "of %s, only data in the %s least significant bits will be kept" ) - log.warning(msg, bit_depth, self.image.data.dtype) + log.warning(msg, self.image.data.dtype, bit_depth) else: if self.image.data.dtype == np.uint8: bit_depth = 8 @@ -61,8 +61,8 @@ def __init__(self, image: ImageModel) -> None: target_bit_depth = 16 target_dtype = np.uint16 - self.image.data *= 2 ** (target_bit_depth - bit_depth) self.image.data = self.image.data.astype(target_dtype) + self.image.data *= 2 ** (target_bit_depth - bit_depth) def create_thumbnail(self) -> None: """ diff --git a/operationsgateway_api/src/records/record.py b/operationsgateway_api/src/records/record.py index c6f0e95c..ef3499a9 100644 --- a/operationsgateway_api/src/records/record.py +++ b/operationsgateway_api/src/records/record.py @@ -376,10 +376,7 @@ async def get_raw_bit_depth( [f"channels.{channel_name}.metadata.bit_depth"], ) metadata = record_dict["channels"][channel_name]["metadata"] - if "bit_depth" in metadata: - raw_bit_depth = metadata["bit_depth"] - else: - raw_bit_depth = None + raw_bit_depth = metadata.get("bit_depth") # May be None return raw_bit_depth @@ -634,16 +631,10 @@ async def _extract_variable( ) img_src = PILImage.open(image_bytes) img_array = np.array(img_src) - if raw_bit_depth in (None, 8, 16): - # If we don't know the original bit depth, or it exactly matches a - # storage depth, no shift is possible/needed - return img_array - elif raw_bit_depth < 8: - # Bit depths < 8 would have been stored as 8 bit, so shift back to raw - return img_array / 2 ** (8 - raw_bit_depth) - else: - # Bit depths > 8 would have been stored as 16 bit, so shift back to raw - return img_array / 2 ** (16 - raw_bit_depth) + return Record._bit_shift_to_raw( + img_array=img_array, + raw_bit_depth=raw_bit_depth, + ) elif channel_dtype == "waveform": waveform_path = channel_value["waveform_path"] @@ -657,6 +648,32 @@ async def _extract_variable( else: return channel_value["data"] + @staticmethod + def _bit_shift_to_raw( + img_array: np.ndarray, + raw_bit_depth: "int | None", + ) -> np.ndarray: + """Shift the bits of a stored image back from most significant to original + positions, so functions are applied to the raw pixel values. + + Args: + img_array (np.ndarray): Stored image as a np.ndarray. + raw_bit_depth (int | None): Original specified bit depth of the raw data. + + Returns: + np.ndarray: Input image with the bits shifted to their original position. + """ + if raw_bit_depth in (None, 8, 16): + # If we don't know the original bit depth, or it exactly matches a + # storage depth, no shift is possible/needed + return img_array + elif raw_bit_depth < 8: + # Bit depths < 8 would have been stored as 8 bit, so shift back to raw + return img_array / 2 ** (8 - raw_bit_depth) + else: + # Bit depths > 8 would have been stored as 16 bit, so shift back to raw + return img_array / 2 ** (16 - raw_bit_depth) + @staticmethod def _parse_function_results( record: dict, diff --git a/test/images/test_false_colour_handler.py b/test/images/test_false_colour_handler.py index fdc545b0..edef5bbb 100644 --- a/test/images/test_false_colour_handler.py +++ b/test/images/test_false_colour_handler.py @@ -1,5 +1,6 @@ import pytest +from operationsgateway_api.src.exceptions import QueryParameterError from operationsgateway_api.src.records.false_colour_handler import FalseColourHandler @@ -70,3 +71,13 @@ def test_pixel_limits( ) assert vmin == vmin_expected[bits_per_pixel] assert vmax == vmax_expected[bits_per_pixel] + + def test_pixel_limits_error(self): + with pytest.raises(QueryParameterError) as e: + FalseColourHandler.pixel_limits( + storage_bit_depth=16, + lower_level=0, + upper_level=512, + limit_bit_depth=8, + ) + assert "upper_level must be less than 2**limit_bit_depth" in e.exconly() diff --git a/test/images/test_image.py b/test/images/test_image.py index 1daaf37b..1181c926 100644 --- a/test/images/test_image.py +++ b/test/images/test_image.py @@ -1,5 +1,6 @@ import base64 from io import BytesIO +import logging import os from unittest.mock import patch @@ -189,11 +190,13 @@ def test_extract_metadata( "operationsgateway_api.src.records.echo_interface.EchoInterface" ".upload_file_object", ) - def test_valid_upload_image(self, mock_upload_file_object, _): + @pytest.mark.parametrize(["bit_depth"], [pytest.param(None), pytest.param(8)]) + def test_valid_upload_image(self, mock_upload_file_object, _, bit_depth: int): test_image = Image( ImageModel( path=self.test_image_path, data=np.ones(shape=(300, 300), dtype=np.uint8), + bit_depth=bit_depth, ), ) Image.upload_image(test_image) @@ -201,7 +204,19 @@ def test_valid_upload_image(self, mock_upload_file_object, _): assert mock_upload_file_object.call_count == 1 assert len(mock_upload_file_object.call_args.args) == 2 - assert isinstance(mock_upload_file_object.call_args.args[0], BytesIO) + uploaded_bytes_io = mock_upload_file_object.call_args.args[0] + assert isinstance(uploaded_bytes_io, BytesIO) + + uploaded_bytes_io.seek(0) + uploaded_bytes = uploaded_bytes_io.read() + s_bit_offset = uploaded_bytes.find(b"sBIT") + if bit_depth is None: + assert s_bit_offset == -1 + else: + assert s_bit_offset != -1 + s_bit = uploaded_bytes[s_bit_offset + 4 : s_bit_offset + 5] + assert int.from_bytes(s_bit, byteorder="big") == bit_depth + assert ( mock_upload_file_object.call_args.args[1] == f"images/{self.test_image_path}" @@ -345,3 +360,113 @@ async def test_invalid_get_image(self, _, __, expected_exception, record_count): def test_get_relative_path(self): test_path = Image.get_relative_path("20220408165857", "N_INP_NF_IMAGE") assert test_path == "20220408165857/N_INP_NF_IMAGE.png" + + @pytest.mark.parametrize( + ["data", "bit_depth", "record_tuples", "dtype", "value"], + [ + pytest.param( + np.ones(1, np.uint8) * 255, + 6, + [], + np.uint8, + 252, + id="Expect to shift up by 2 bits, 1111 1111 -> 1111 1100", + ), + pytest.param( + np.ones(1, np.uint16) * 65535, + 6, + [ + ( + "root", + logging.WARNING, + ( + "Specified bit depth is lower than actual bit depth with " + "dtype of uint16, only data in the 6 least significant " + "bits will be kept" + ), + ), + ], + np.uint8, + 252, + id=( + "Expect to shift up by 2 bits, 1111 1111 1111 1111 -> 1111 1100, " + "and warn only lower bits are kept" + ), + ), + pytest.param( + np.ones(1, np.uint8) * 255, + 8, + [], + np.uint8, + 255, + id="Expect no change", + ), + pytest.param( + np.ones(1, np.uint16) * 255, + 8, + [ + ( + "root", + logging.WARNING, + ( + "Specified bit depth is lower than actual bit depth with " + "dtype of uint16, only data in the 8 least significant " + "bits will be kept" + ), + ), + ], + np.uint8, + 255, + id=("Expect no change, and warn that only lower bits are kept"), + ), + pytest.param( + np.ones(1, np.uint8) * 255, + 12, + [], + np.uint16, + 4080, + id="Expect to shift up 4 bits, 1111 1111 -> 0000 1111 1111 0000", + ), + pytest.param( + np.ones(1, np.uint16) * 65535, + 12, + [], + np.uint16, + 65520, + id=( + "Expect to shift up by 4 bits, " + "1111 1111 1111 1111 -> 1111 1111 1111 0000" + ), + ), + pytest.param( + np.ones(1, np.uint8) * 255, + 16, + [], + np.uint16, + 255, + id="Expect no change", + ), + pytest.param( + np.ones(1, np.uint16) * 65535, + 16, + [], + np.uint16, + 65535, + id="Expect no change", + ), + ], + ) + def test_bit_depth( + self, + data: np.ndarray, + bit_depth: int, + record_tuples: "list[tuple[str, int, str]]", + dtype: type, + value: int, + caplog, + ): + image_model = ImageModel(path="", data=data, bit_depth=bit_depth) + image = Image(image_model) + assert caplog.record_tuples == record_tuples + assert image.image.data.dtype == dtype + assert image.image.data[0] == value diff --git a/test/records/ingestion/create_test_hdf.py b/test/records/ingestion/create_test_hdf.py index cda618d6..7d396b8a 100644 --- a/test/records/ingestion/create_test_hdf.py +++ b/test/records/ingestion/create_test_hdf.py @@ -268,6 +268,8 @@ async def create_test_hdf_file( # noqa: C901 pm_201_fe_cam_1.attrs.create("y_pixel_units", 346) else: pm_201_fe_cam_1.attrs.create("y_pixel_units", "µm") + if "bit_depth" in image and image["bit_depth"] == "invalid": + pm_201_fe_cam_1.attrs.create("bit_depth", "12") else: pm_201_fe_cam_1.attrs.create("exposure_time_s", 0.001) pm_201_fe_cam_1.attrs.create("gain", 5.5) diff --git a/test/records/ingestion/test_channel.py b/test/records/ingestion/test_channel.py index 08b66fb3..5a02cf52 100644 --- a/test/records/ingestion/test_channel.py +++ b/test/records/ingestion/test_channel.py @@ -499,6 +499,11 @@ async def test_required_attribute( ], id="Mixed invalid optional attributes", ), + pytest.param( + {"image": {"bit_depth": "invalid"}}, + [{"PM-201-FE-CAM-1": "bit_depth attribute has wrong datatype"}], + id="Invalid bit_depth", + ), ], ) @pytest.mark.asyncio diff --git a/test/records/test_record.py b/test/records/test_record.py index 73d45948..99649a3e 100644 --- a/test/records/test_record.py +++ b/test/records/test_record.py @@ -600,3 +600,14 @@ async def test_apply_functions_missing_channel(self): assert "b" not in record["channels"] # Skip, a undefined assert "c" in record["channels"] # Has no dependencies, so should be returned assert record["channels"]["c"] == expected + + @pytest.mark.parametrize( + ["img_array", "raw_bit_depth"], + [ + pytest.param(np.ones(1, dtype=np.int32) * 4, 6), + pytest.param(np.ones(1, dtype=np.int32) * 16, 12), + ], + ) + def test_bit_shift_to_raw(self, img_array: np.ndarray, raw_bit_depth: int): + img = Record._bit_shift_to_raw(img_array=img_array, raw_bit_depth=raw_bit_depth) + assert img[0] == 1 From cfd1d0d9a38067c79d65ac214fed8b614d4ab54b Mon Sep 17 00:00:00 2001 From: Patrick Austin Date: Mon, 27 Jan 2025 14:32:50 +0000 Subject: [PATCH 3/9] Increase lower_level, upper_level max to 65535 to allow 16 bit levels --- .../src/records/export_handler.py | 7 +++++-- operationsgateway_api/src/routes/export.py | 16 ++++++++++++++-- operationsgateway_api/src/routes/images.py | 8 ++++---- test/endpoints/test_get_colour_bar_image.py | 6 +++--- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/operationsgateway_api/src/records/export_handler.py b/operationsgateway_api/src/records/export_handler.py index 1394f7d9..87cf4bb7 100644 --- a/operationsgateway_api/src/records/export_handler.py +++ b/operationsgateway_api/src/records/export_handler.py @@ -25,6 +25,7 @@ def __init__( projection: List[str], lower_level: int, upper_level: int, + limit_bit_depth: int, colourmap_name: str, functions: "list[dict[str, str]]", export_scalars: bool, @@ -40,6 +41,7 @@ def __init__( self.projection = projection self.lower_level = lower_level self.upper_level = upper_level + self.limit_bit_depth = limit_bit_depth self.colourmap_name = colourmap_name self.export_scalars = export_scalars self.export_images = export_images @@ -68,6 +70,7 @@ def original_image(self) -> bool: return ( self.lower_level == 0 and self.upper_level == 255 + and self.limit_bit_depth == 8 and self.colourmap_name is None ) @@ -97,7 +100,7 @@ async def process_records(self) -> None: original_image=self.original_image, lower_level=self.lower_level, upper_level=self.upper_level, - limit_bit_depth=8, # Assume the limit bit depth for the export + limit_bit_depth=self.limit_bit_depth, colourmap_name=self.colourmap_name, return_thumbnails=False, ) @@ -270,7 +273,7 @@ async def _add_image_to_zip( original_image=self.original_image, lower_level=self.lower_level, upper_level=self.upper_level, - limit_bit_depth=8, # Assume the limit bit depth for the export + limit_bit_depth=self.limit_bit_depth, colourmap_name=self.colourmap_name, ) self.zip_file.writestr( diff --git a/operationsgateway_api/src/routes/export.py b/operationsgateway_api/src/routes/export.py index 8e6fab4a..64f29599 100644 --- a/operationsgateway_api/src/routes/export.py +++ b/operationsgateway_api/src/routes/export.py @@ -61,13 +61,24 @@ async def export_records( 0, description="The lower level threshold for false colour (0-255)", ge=0, - le=255, + le=65535, ), upper_level: Optional[int] = Query( 255, description="The upper level threshold for false colour (0-255)", ge=0, - le=255, + le=65535, + ), + limit_bit_depth: int = Query( + 8, + description=( + "The bit depth to which `lower_level` and `upper_level` are relative. This " + "can be any value as long as it is consistent with the provided levels, " + "but is intended to reflect the original bit depth of the raw data, so " + "meaningful values can be displayed and set in the frontend." + ), + ge=0, + le=16, ), colourmap_name: Optional[str] = Query( None, @@ -141,6 +152,7 @@ async def export_records( projection, lower_level, upper_level, + limit_bit_depth, colourmap_name, functions, export_scalars, diff --git a/operationsgateway_api/src/routes/images.py b/operationsgateway_api/src/routes/images.py index dcd88dca..5a27d2c2 100644 --- a/operationsgateway_api/src/routes/images.py +++ b/operationsgateway_api/src/routes/images.py @@ -50,13 +50,13 @@ async def get_full_image( 255, description="The upper level threshold for false colour (0-255)", ge=0, - le=255, + le=65535, ), lower_level: Optional[int] = Query( 0, description="The lower level threshold for false colour (0-255)", ge=0, - le=255, + le=65535, ), limit_bit_depth: int = Query( 8, @@ -195,13 +195,13 @@ async def get_colourbar_image( 255, description="The upper level threshold for false colour (0-255)", ge=0, - le=255, + le=65535, ), lower_level: Optional[int] = Query( 0, description="The lower level threshold for false colour (0-255)", ge=0, - le=255, + le=65535, ), limit_bit_depth: int = Query( 8, diff --git a/test/endpoints/test_get_colour_bar_image.py b/test/endpoints/test_get_colour_bar_image.py index e19dd029..5d61486e 100644 --- a/test/endpoints/test_get_colour_bar_image.py +++ b/test/endpoints/test_get_colour_bar_image.py @@ -59,16 +59,16 @@ class TestGetColourBarImage: False, 422, # pydantic error None, - id="Client error due to lower_level not in range 1 to 255", + id="Client error due to lower_level not in range 1 to 65535", ), pytest.param( None, - 300, + 65536, None, False, 422, # pydantic error None, - id="Client error due to upper_level not in range 1 to 255", + id="Client error due to upper_level not in range 1 to 65535", ), pytest.param( 100, From 55211efb4a3e67d91496a06106c1f1018f7e2354 Mon Sep 17 00:00:00 2001 From: Patrick Austin Date: Tue, 28 Jan 2025 14:08:16 +0000 Subject: [PATCH 4/9] Make np.integer -> int cast explicit for bit_depth --- operationsgateway_api/src/models.py | 21 +++++++++++ .../src/records/ingestion/channel_checks.py | 2 +- test/test_models.py | 37 +++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 test/test_models.py diff --git a/operationsgateway_api/src/models.py b/operationsgateway_api/src/models.py index ba618f79..e651de10 100644 --- a/operationsgateway_api/src/models.py +++ b/operationsgateway_api/src/models.py @@ -77,6 +77,27 @@ class ImageChannelMetadataModel(BaseModel): y_pixel_units: Optional[Union[str, Any]] = None bit_depth: Optional[Union[int, Any]] = None + @field_validator("bit_depth") + def validate_bit_depth(bit_depth: "int | Any | None") -> "int | Any | None": + """ + Ensure that we do not attempt to persist a np.integer by the use of Any. + While the value from the hdf5 file will (at time of writing) be this type, it + cannot be sent to Mongo in the model_dump as it is not a valid JSON type. + + Oddly, this only needs to be done for integers - floats behave as expected, and + Pydantic casts np.floating to float upon __init__. + + Args: + bit_depth (int | Any | None): Value for bit depth, possible a np.integer + + Returns: + int | Any | None: Value for bit depth, definitely not a np.integer + """ + if isinstance(bit_depth, np.integer): + return int(bit_depth) + else: + return bit_depth + class ImageChannelModel(BaseModel): metadata: ImageChannelMetadataModel diff --git a/operationsgateway_api/src/records/ingestion/channel_checks.py b/operationsgateway_api/src/records/ingestion/channel_checks.py index 2b9b7aa4..41e68645 100644 --- a/operationsgateway_api/src/records/ingestion/channel_checks.py +++ b/operationsgateway_api/src/records/ingestion/channel_checks.py @@ -253,7 +253,7 @@ def image_metadata_checks(cls, key, value_dict, rejected_channels): if ( "bit_depth" in value_dict and value_dict["bit_depth"] is not None - and not isinstance(value_dict["bit_depth"], int) + and not isinstance(value_dict["bit_depth"], (int, np.integer)) ): rejected_channels.append({key: "bit_depth attribute has wrong datatype"}) diff --git a/test/test_models.py b/test/test_models.py new file mode 100644 index 00000000..eea59693 --- /dev/null +++ b/test/test_models.py @@ -0,0 +1,37 @@ +import numpy as np +import pytest +from operationsgateway_api.src.models import ImageChannelMetadataModel + + +class TestModels: + @pytest.mark.parametrize( + ["gain", "gain_type", "bit_depth", "bit_depth_type"], + [ + pytest.param(1.0, float, 1, int, id="Python raw types"), + pytest.param(np.float64(1), float, np.int64(1), int, id="Numpy dtypes"), + pytest.param("1", str, "1", str, id="Castable strings"), + pytest.param("one", str, "one", str, id="Un-castable strings"), + ], + ) + def test_image_channel_metadata_model( + self, + gain: "float | np.float64", + gain_type: type, + bit_depth: "int | np.int64", + bit_depth_type: type, + ): + """ + Ensure that ints and floats are handled consistently by the __init__ of the + model. Python dtypes should be case to raw types (to allow them to be dumped + into Mongo) but even castable strings should not be accepted so we can raise a + channel check error when the time comes. + """ + model = ImageChannelMetadataModel( + channel_dtype="image", + gain=gain, + bit_depth=bit_depth, + ) + gain_correct = isinstance(model.gain, gain_type) + bit_depth_correct = isinstance(model.bit_depth, bit_depth_type) + assert gain_correct, f"{type(model.gain)} != {gain_type}" + assert bit_depth_correct, f"{type(model.bit_depth)} != {bit_depth_type}" From a566fa66083a9caa7d0fe325dbc18b1e4a086fe0 Mon Sep 17 00:00:00 2001 From: Patrick Austin Date: Tue, 28 Jan 2025 14:17:03 +0000 Subject: [PATCH 5/9] Apply linting to models.py, test_models.py --- operationsgateway_api/src/models.py | 3 ++- test/test_models.py | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/operationsgateway_api/src/models.py b/operationsgateway_api/src/models.py index e651de10..1a0182b0 100644 --- a/operationsgateway_api/src/models.py +++ b/operationsgateway_api/src/models.py @@ -78,7 +78,8 @@ class ImageChannelMetadataModel(BaseModel): bit_depth: Optional[Union[int, Any]] = None @field_validator("bit_depth") - def validate_bit_depth(bit_depth: "int | Any | None") -> "int | Any | None": + @classmethod + def validate_bit_depth(cls, bit_depth: "int | Any | None") -> "int | Any | None": """ Ensure that we do not attempt to persist a np.integer by the use of Any. While the value from the hdf5 file will (at time of writing) be this type, it diff --git a/test/test_models.py b/test/test_models.py index eea59693..1e3eaec5 100644 --- a/test/test_models.py +++ b/test/test_models.py @@ -1,5 +1,6 @@ import numpy as np import pytest + from operationsgateway_api.src.models import ImageChannelMetadataModel From bbf53cb86b1936ec58cc6252e2fadec16e1a2099 Mon Sep 17 00:00:00 2001 From: Patrick Austin Date: Wed, 29 Jan 2025 09:59:09 +0000 Subject: [PATCH 6/9] Store and use max bit depth for functions --- operationsgateway_api/src/records/record.py | 59 +++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/operationsgateway_api/src/records/record.py b/operationsgateway_api/src/records/record.py index ef3499a9..f219779c 100644 --- a/operationsgateway_api/src/records/record.py +++ b/operationsgateway_api/src/records/record.py @@ -518,6 +518,7 @@ async def _apply_function( variable_transformer.evaluate(function["expression"]) variables = variable_transformer.variables channels_to_fetch = set() + bit_depths = [] log.debug("Attempting to extract %s from %s", variables, record["channels"]) for variable in variables: @@ -526,6 +527,7 @@ async def _apply_function( record["_id"], variable, record["channels"][variable], + bit_depths=bit_depths, ) else: channels_to_fetch.add(variable) @@ -536,6 +538,7 @@ async def _apply_function( variable_data, channels_to_fetch, skip_functions=variable_transformer.skip_functions, + bit_depths=bit_depths, ) if missing_channels: # Remove any missing channels so we don't skip future functions which @@ -556,6 +559,7 @@ async def _apply_function( result=result, return_thumbnails=return_thumbnails, truncate=truncate, + bit_depths=bit_depths, ) variable_data[function["name"]] = result @@ -565,6 +569,7 @@ async def _fetch_channels( variable_data: dict, channels_to_fetch: "set[str]", skip_functions: "set[str]", + bit_depths: "list[int]", ) -> "set[str]": """Fetches `channels_to_fetch`, returning known channels missing for this record and raising an exception if the channel is not known at all.""" @@ -594,6 +599,7 @@ async def _fetch_channels( record["_id"], name, channel_value, + bit_depths=bit_depths, ) @staticmethod @@ -601,6 +607,7 @@ async def _extract_variable( record_id: str, name: str, channel_value: dict, + bit_depths: "list[int]", ) -> "np.ndarray | WaveformVariable | float": """ Extracts and returns the relevant data from `channel_value`, handling @@ -620,6 +627,10 @@ async def _extract_variable( channel_name=name, channel_value=channel_value, ) + if raw_bit_depth is not None: + # Modify in place to store for each channel, getting around static func + bit_depths.append(raw_bit_depth) + image_bytes = await Image.get_image( record_id=record_id, channel_name=name, @@ -674,6 +685,33 @@ def _bit_shift_to_raw( # Bit depths > 8 would have been stored as 16 bit, so shift back to raw return img_array / 2 ** (16 - raw_bit_depth) + @staticmethod + def _bit_shift_to_storage( + img_array: np.ndarray, + raw_bit_depth: "int | None", + ) -> "tuple[np.ndarray, int]": + """Shift the bits of a calculated image from numerically accurate positions to + most significant bits for display/storage. + + Args: + img_array (np.ndarray): Calculated image as a np.ndarray. + raw_bit_depth (int | None): Original specified bit depth of the raw data. + + Returns: + tuple[np.ndarray, int]: + Input image with the bits shifted to storage/display positions, + and the value of this storage bit depth. + """ + if raw_bit_depth in (8, 16): + # If bit depth exactly matches a storage depth, no shift is needed + return img_array, raw_bit_depth + elif raw_bit_depth < 8: + # Bit depths < 8 would have been stored as 8 bit, so shift up to storage + return img_array.astype(np.uint8) * 2 ** (8 - raw_bit_depth), 8 + else: + # Bit depths > 8 would have been stored as 16 bit, so shift up to storage + return img_array.astype(np.uint16) * 2 ** (16 - raw_bit_depth), 16 + @staticmethod def _parse_function_results( record: dict, @@ -684,6 +722,7 @@ def _parse_function_results( colourmap_name: str, function_name: str, result: "np.ndarray | WaveformVariable | np.float64", + bit_depths: "list[int]", return_thumbnails: bool = True, truncate: bool = False, ) -> None: @@ -703,6 +742,7 @@ def _parse_function_results( colourmap_name=colourmap_name, return_thumbnails=return_thumbnails, truncate=truncate, + bit_depths=bit_depths, ) elif isinstance(result, WaveformVariable): @@ -737,13 +777,26 @@ def _parse_image_result( colourmap_name: str, return_thumbnails: bool, truncate: bool, + bit_depths: "list[int]", ) -> dict: """Parses a numpy ndarray and returns image bytes, either for a thumbnail or full image. """ - # We do not track the bit depth of inputs, so keep maximum depth to - # avoid losing information - storage_bit_depth = 16 + if len(bit_depths) == 0: + # We have no information about input bit depths, so set to max supported + # This will not lose any information, but may make the image very dark + overall_bit_depth = 16 + else: + # Otherwise, take the highest depth encountered. There may be more than one + # if the function depends on multiple channels with different depths, + # in which case, we should try and store all information which means + # choosing the highest bit depth needed + overall_bit_depth = max(bit_depths) + + result, storage_bit_depth = Record._bit_shift_to_storage( + img_array=result, + raw_bit_depth=overall_bit_depth, + ) if return_thumbnails: metadata = { "channel_dtype": "image", From 98b552000c6f8afce9be02581736ff94ab67a0fa Mon Sep 17 00:00:00 2001 From: Patrick Austin Date: Fri, 7 Feb 2025 17:25:04 +0000 Subject: [PATCH 7/9] Add limit_bit_depth, 12 bit example to Postman --- util/og_api_postman_collection.json | 76 ++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/util/og_api_postman_collection.json b/util/og_api_postman_collection.json index 8e3f0cb5..0f15d611 100644 --- a/util/og_api_postman_collection.json +++ b/util/og_api_postman_collection.json @@ -1,6 +1,6 @@ { "info": { - "_postman_id": "481d999a-cda0-4e7d-801c-efcee7b4f09d", + "_postman_id": "fa28d10a-2892-4814-b257-65fb06897008", "name": "OperationsGateway API", "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json", "_exporter_id": "33242392" @@ -962,6 +962,11 @@ "value": "10", "disabled": true }, + { + "key": "limit_bit_depth", + "value": "8", + "disabled": true + }, { "key": "colourmap_name", "value": "jet_r", @@ -976,6 +981,65 @@ }, "response": [] }, + { + "name": "Get 12 bit image", + "request": { + "auth": { + "type": "bearer", + "bearer": [ + { + "key": "token", + "value": "{{og-access-token}}", + "type": "string" + } + ] + }, + "method": "GET", + "header": [], + "url": { + "raw": "{{og-api}}/images/20230606120000/CM-202-CVC-CAM-1?lower_level=512&upper_level=1023&limit_bit_depth=12&colourmap_name=jet_r", + "host": [ + "{{og-api}}" + ], + "path": [ + "images", + "20230606120000", + "CM-202-CVC-CAM-1" + ], + "query": [ + { + "key": "original_image", + "value": "true", + "disabled": true + }, + { + "key": "lower_level", + "value": "512", + "description": "Value relative to a 12 bit max of 4095" + }, + { + "key": "upper_level", + "value": "1023", + "description": "Value relative to a 12 bit max of 4095" + }, + { + "key": "limit_bit_depth", + "value": "12" + }, + { + "key": "colourmap_name", + "value": "jet_r" + }, + { + "key": "functions", + "value": "{\"name\": \"a\", \"expression\": \"CM-202-CVC-CAM-1 %2B 1280\"}", + "disabled": true + } + ] + } + }, + "response": [] + }, { "name": "Get Crosshair Intensity", "request": { @@ -1053,6 +1117,11 @@ "value": "175", "disabled": true }, + { + "key": "limit_bit_depth", + "value": "8", + "disabled": true + }, { "key": "colourmap_name", "value": "jet_r", @@ -1749,6 +1818,11 @@ "value": "15", "disabled": true }, + { + "key": "limit_bit_depth", + "value": "8", + "disabled": true + }, { "key": "colourmap_name", "value": "jet_r", From 5b700bb36147cdf4445eb2f8b8f060abaf780f95 Mon Sep 17 00:00:00 2001 From: Patrick Austin Date: Mon, 10 Feb 2025 15:32:44 +0000 Subject: [PATCH 8/9] Add explicit 0, 24 bit depth tests and prevent dtype mismatch --- operationsgateway_api/src/records/image.py | 9 ++- test/images/test_image.py | 73 +++++++++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/operationsgateway_api/src/records/image.py b/operationsgateway_api/src/records/image.py index a2a1c152..6e38a756 100644 --- a/operationsgateway_api/src/records/image.py +++ b/operationsgateway_api/src/records/image.py @@ -45,6 +45,11 @@ def __init__(self, image: ImageModel) -> None: "of %s, only data in the %s least significant bits will be kept" ) log.warning(msg, self.image.data.dtype, bit_depth) + elif bit_depth > 16: + log.warning( + "Specified bit depth is higher than the max supported depth of 16, " + "only data in the 16 most significant bits will be kept", + ) else: if self.image.data.dtype == np.uint8: bit_depth = 8 @@ -62,7 +67,9 @@ def __init__(self, image: ImageModel) -> None: target_dtype = np.uint16 self.image.data = self.image.data.astype(target_dtype) - self.image.data *= 2 ** (target_bit_depth - bit_depth) + shifted_data = self.image.data * 2 ** (target_bit_depth - bit_depth) + # Negative shifts may result in a float output, so cast the type again + self.image.data = shifted_data.astype(target_dtype) def create_thumbnail(self) -> None: """ diff --git a/test/images/test_image.py b/test/images/test_image.py index 1181c926..19b41bfb 100644 --- a/test/images/test_image.py +++ b/test/images/test_image.py @@ -364,6 +364,35 @@ def test_get_relative_path(self): @pytest.mark.parametrize( ["data", "bit_depth", "record_tuples", "dtype", "value"], [ + pytest.param( + np.ones(1, np.uint8) * 255, + 0, + [], + np.uint8, + 0, + id="Expect to shift up by 8 bits, 1111 1111 -> 0000 0000", + ), + pytest.param( + np.ones(1, np.uint16) * 65535, + 0, + [ + ( + "root", + logging.WARNING, + ( + "Specified bit depth is lower than actual bit depth with " + "dtype of uint16, only data in the 0 least significant " + "bits will be kept" + ), + ), + ], + np.uint8, + 0, + id=( + "Expect to shift up by 8 bits, 1111 1111 1111 1111 -> 0000 0000, " + "and warn only lower bits are kept" + ), + ), pytest.param( np.ones(1, np.uint8) * 255, 6, @@ -417,7 +446,7 @@ def test_get_relative_path(self): ], np.uint8, 255, - id=("Expect no change, and warn that only lower bits are kept"), + id="Expect no change, and warn that only lower bits are kept", ), pytest.param( np.ones(1, np.uint8) * 255, @@ -454,6 +483,48 @@ def test_get_relative_path(self): 65535, id="Expect no change", ), + pytest.param( + np.ones(1, np.uint8) * 255, + 24, + [ + ( + "root", + logging.WARNING, + ( + "Specified bit depth is higher than the max supported " + "depth of 16, only data in the 16 most significant bits " + "will be kept" + ), + ), + ], + np.uint16, + 0, + id=( + "Expect to shift down by 8 bits, 1111 1111 -> 0000 0000, " + "and warn only upper bits are kept" + ), + ), + pytest.param( + np.ones(1, np.uint16) * 65535, + 24, + [ + ( + "root", + logging.WARNING, + ( + "Specified bit depth is higher than the max supported " + "depth of 16, only data in the 16 most significant bits " + "will be kept" + ), + ), + ], + np.uint16, + 255, + id=( + "Expect to shift down by 8 bits, 1111 1111 1111 1111 -> " + "0000 0000 1111 1111, and warn only upper bits are kept" + ), + ), ], ) def test_bit_depth( From 9d7c47002635ea94bfe3518e6fdd1f5517aaf975 Mon Sep 17 00:00:00 2001 From: Patrick Austin Date: Mon, 10 Feb 2025 16:02:18 +0000 Subject: [PATCH 9/9] Only support 0 < sbit <= 16 in upload_image --- operationsgateway_api/src/records/image.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/operationsgateway_api/src/records/image.py b/operationsgateway_api/src/records/image.py index 6e38a756..7a630512 100644 --- a/operationsgateway_api/src/records/image.py +++ b/operationsgateway_api/src/records/image.py @@ -122,9 +122,10 @@ def upload_image(input_image: Image) -> Optional[str]: image_bytes = BytesIO() try: image = PILImage.fromarray(input_image.image.data) - if input_image.image.bit_depth is not None: + bit_depth = input_image.image.bit_depth + if bit_depth is not None and 0 < bit_depth <= 16: info = PngImagePlugin.PngInfo() - sbit = input_image.image.bit_depth.to_bytes(1, byteorder="big") + sbit = bit_depth.to_bytes(1, byteorder="big") info.add(b"sBIT", sbit) image.save(image_bytes, format="PNG", pnginfo=info) else: