Bitmask input array dimension check #454
Conversation
|
Walkthrough: The changes primarily focus on enhancing error handling and improving clarity in the code. They introduce checks for input validation in Changes:
NotesChat with
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
encord/objects/bitmask.py
Outdated
|
|
||
| @staticmethod | ||
| def _from_array(source: Any) -> BitmaskCoordinates.EncodedBitmask: | ||
| if source is not None: | ||
| if hasattr(source, "__array_interface__"): | ||
| arr = source.__array_interface__ | ||
| data_type = arr["typestr"] | ||
| data = arr["data"] | ||
| shape = arr["shape"] | ||
|
|
||
| if data_type != "|b1": | ||
| raise EncordException( | ||
| "Bitmask should be an array of boolean values. For numpy array call .astype(bool)." | ||
| ) | ||
| if source is not None and hasattr(source, "__array_interface__"): | ||
| arr = source.__array_interface__ | ||
| data_type = arr["typestr"] | ||
| data = arr["data"] | ||
| shape = arr["shape"] | ||
|
|
||
| if len(shape) != 2: | ||
| raise EncordException("Bitmask should be a 2-dimensional array.") | ||
|
|
||
| if data_type != "|b1": | ||
| raise EncordException( | ||
| "Bitmask should be an array of boolean values. For numpy array call .astype(bool)." | ||
| ) | ||
|
|
||
| raw_data = data if isinstance(data, bytes) else source.tobytes() | ||
| raw_data = data if isinstance(data, bytes) else source.tobytes() | ||
|
|
||
| rle = _mask_to_rle(raw_data) | ||
| rle_string = _rle_to_string(rle) | ||
| rle = _mask_to_rle(raw_data) | ||
| rle_string = _rle_to_string(rle) | ||
|
|
||
| return BitmaskCoordinates.EncodedBitmask( | ||
| top=0, left=0, height=shape[0], width=shape[1], rle_string=rle_string | ||
| ) | ||
| return BitmaskCoordinates.EncodedBitmask( | ||
| top=0, left=0, height=shape[0], width=shape[1], rle_string=rle_string | ||
| ) | ||
|
|
||
| raise EncordException(f"Can't import bitmask from {source.__class__}") | ||
|
|
There was a problem hiding this comment.
The changes made in this hunk are logically sound and improve the error handling by adding a check for the dimensionality of the input array. The error messages are clear and provide useful information to the user about what is expected. The code is also more maintainable as it checks for potential issues before they can cause problems later on.
| @staticmethod | |
| def _from_array(source: Any) -> BitmaskCoordinates.EncodedBitmask: | |
| if source is not None: | |
| if hasattr(source, "__array_interface__"): | |
| arr = source.__array_interface__ | |
| data_type = arr["typestr"] | |
| data = arr["data"] | |
| shape = arr["shape"] | |
| if data_type != "|b1": | |
| raise EncordException( | |
| "Bitmask should be an array of boolean values. For numpy array call .astype(bool)." | |
| ) | |
| if source is not None and hasattr(source, "__array_interface__"): | |
| arr = source.__array_interface__ | |
| data_type = arr["typestr"] | |
| data = arr["data"] | |
| shape = arr["shape"] | |
| if len(shape) != 2: | |
| raise EncordException("Bitmask should be a 2-dimensional array.") | |
| if data_type != "|b1": | |
| raise EncordException( | |
| "Bitmask should be an array of boolean values. For numpy array call .astype(bool)." | |
| ) | |
| raw_data = data if isinstance(data, bytes) else source.tobytes() | |
| raw_data = data if isinstance(data, bytes) else source.tobytes() | |
| rle = _mask_to_rle(raw_data) | |
| rle_string = _rle_to_string(rle) | |
| rle = _mask_to_rle(raw_data) | |
| rle_string = _rle_to_string(rle) | |
| return BitmaskCoordinates.EncodedBitmask( | |
| top=0, left=0, height=shape[0], width=shape[1], rle_string=rle_string | |
| ) | |
| return BitmaskCoordinates.EncodedBitmask( | |
| top=0, left=0, height=shape[0], width=shape[1], rle_string=rle_string | |
| ) | |
| raise EncordException(f"Can't import bitmask from {source.__class__}") | |
| @staticmethod | |
| def _from_array(source: Any) -> BitmaskCoordinates.EncodedBitmask: | |
| if source is not None and hasattr(source, "__array_interface__"): | |
| arr = source.__array_interface__ | |
| data_type = arr["typestr"] | |
| data = arr["data"] | |
| shape = arr["shape"] | |
| if len(shape) != 2: | |
| raise EncordException("Bitmask should be a 2-dimensional array.") | |
| if data_type != "|b1": | |
| raise EncordException( | |
| "Bitmask should be an array of boolean values. For numpy array call .astype(bool)." | |
| ) | |
| raw_data = data if isinstance(data, bytes) else source.tobytes() | |
| rle = _mask_to_rle(raw_data) | |
| rle_string = _rle_to_string(rle) | |
| return BitmaskCoordinates.EncodedBitmask( | |
| top=0, left=0, height=shape[0], width=shape[1], rle_string=rle_string | |
| ) | |
| raise EncordException(f"Can't import bitmask from {source.__class__}") |
| if source is None: | ||
| raise EncordException("Bitmask can't be created from None") | ||
|
|
There was a problem hiding this comment.
The check for None is good, but it might be more Pythonic to use if source is None instead of if source == None. The is operator checks for identity, not equality, which is more appropriate when comparing with None.
| if source is None: | |
| raise EncordException("Bitmask can't be created from None") | |
| if source is None: | |
| raise EncordException("Bitmask can't be created from None") |
| arr = source.__array_interface__ | ||
| data_type = arr["typestr"] | ||
| data = arr["data"] | ||
| shape = arr["shape"] | ||
|
|
||
| rle = _mask_to_rle(raw_data) | ||
| rle_string = _rle_to_string(rle) | ||
| if len(shape) != 2: | ||
| raise EncordException("Bitmask should be a 2-dimensional array.") |
There was a problem hiding this comment.
The extraction and validation of the array interface are well done. However, the error message in line 151 could be more informative by including the actual dimensions of the input array.
| arr = source.__array_interface__ | |
| data_type = arr["typestr"] | |
| data = arr["data"] | |
| shape = arr["shape"] | |
| rle = _mask_to_rle(raw_data) | |
| rle_string = _rle_to_string(rle) | |
| if len(shape) != 2: | |
| raise EncordException("Bitmask should be a 2-dimensional array.") | |
| if len(shape) != 2: | |
| raise EncordException(f"Bitmask should be a 2-dimensional array, but got {len(shape)}-dimensional array.") |
| if data_type != "|b1": | ||
| raise EncordException("Bitmask should be an array of boolean values. For numpy array call .astype(bool).") | ||
|
|
There was a problem hiding this comment.
The data type check and the corresponding error message are clear and helpful.
| raw_data = data if isinstance(data, bytes) else source.tobytes() | ||
|
|
||
| rle = _mask_to_rle(raw_data) | ||
| rle_string = _rle_to_string(rle) | ||
|
|
||
| return BitmaskCoordinates.EncodedBitmask(top=0, left=0, height=shape[0], width=shape[1], rle_string=rle_string) |
There was a problem hiding this comment.
The conversion of the data to bytes and the creation of the EncodedBitmask object are correctly implemented.
|
|
||
| @staticmethod | ||
| def _from_array(source: Any) -> BitmaskCoordinates.EncodedBitmask: | ||
| if source is not None: | ||
| if hasattr(source, "__array_interface__"): | ||
| arr = source.__array_interface__ | ||
| data_type = arr["typestr"] | ||
| data = arr["data"] | ||
| shape = arr["shape"] | ||
| if source is None: | ||
| raise EncordException("Bitmask can't be created from None") | ||
|
|
||
| if data_type != "|b1": | ||
| raise EncordException( | ||
| "Bitmask should be an array of boolean values. For numpy array call .astype(bool)." | ||
| ) | ||
| if not hasattr(source, "__array_interface__"): | ||
| raise EncordException(f"Can't import bitmask from {source.__class__}") | ||
|
|
||
| raw_data = data if isinstance(data, bytes) else source.tobytes() | ||
| arr = source.__array_interface__ | ||
| data_type = arr["typestr"] | ||
| data = arr["data"] | ||
| shape = arr["shape"] | ||
|
|
||
| rle = _mask_to_rle(raw_data) | ||
| rle_string = _rle_to_string(rle) | ||
| if len(shape) != 2: | ||
| raise EncordException("Bitmask should be a 2-dimensional array.") | ||
|
|
||
| return BitmaskCoordinates.EncodedBitmask( | ||
| top=0, left=0, height=shape[0], width=shape[1], rle_string=rle_string | ||
| ) | ||
| if data_type != "|b1": | ||
| raise EncordException("Bitmask should be an array of boolean values. For numpy array call .astype(bool).") | ||
|
|
||
| raise EncordException(f"Can't import bitmask from {source.__class__}") | ||
| raw_data = data if isinstance(data, bytes) else source.tobytes() |
There was a problem hiding this comment.
The changes made in these lines are good for improving the robustness of the code. The developer has added checks to ensure that the source is not None, it's a 2-dimensional array, and it consists of boolean values only. If any of these conditions are not met, an exception is raised with a clear error message. This will help in debugging and maintaining the code.
| def to_numpy_array(self): | ||
| """ | ||
| Converts the mask to numpy array with dtype bool. | ||
| Converts the mask to a 2D numpy array with dtype bool. | ||
|
|
||
| Numpy needs to be installed for this call to work | ||
| Numpy needs to be installed for this call to work. | ||
| """ | ||
| try: | ||
| import numpy as np # type: ignore[missing-import] | ||
| except ImportError: | ||
| raise EncordException("Numpy is required for .to_numpy call.") | ||
| except ImportError as e: | ||
| raise EncordException("Numpy is required for .to_numpy_array call.") from e | ||
|
|
||
| return np.array(self) |
There was a problem hiding this comment.
The changes made in these lines are also beneficial. The developer has improved the error handling by raising an exception if Numpy is not installed. This makes the code more robust and easier to debug. The change in the docstring from ".to_numpy call" to ".to_numpy_array call" is also a good one as it accurately reflects the method name.
Raise an exception with the clear error message if a provided bitmask is not a 2d array
Summary by CodeRabbit
Bug fix:
bitmask.py.BitmaskCoordinatesclass methods_from_array,from_array, andto_numpy_arrayinbitmask.py.ontology_labels_impl.py.