refactor, improve testing validate, pass pylint

Author: Matt Land

adafruit_imageload/bmp/__init__.py

@@ -56,9 +56,9 @@ def load(file, *, bitmap=None, palette=None):
 
     if colors == 0 and color_depth >= 16:
         raise NotImplementedError("True color BMP unsupported")
-    else:
-        if colors == 0:
-            colors = 2 ** color_depth
-        from . import indexed
-        return indexed.load(file, width, height, data_start, colors, color_depth, bitmap=bitmap,
-                            palette=palette)
+
+    if colors == 0:
+        colors = 2 ** color_depth
+    from . import indexed
+    return indexed.load(file, width, height, data_start, colors, color_depth, bitmap=bitmap,
+                        palette=palette)

adafruit_imageload/pnm/__init__.py

@@ -39,14 +39,37 @@ def load(file, header, *, bitmap=None, palette=None):
     to do the actual data loading.
     Formats P1, P4 have two space padded pieces of information: width and height.
     All other formats have three: width, height, and max color value.
+    This load function will move the file stream pointer to the start of data in all cases.
     """
+    # pylint: disable=too-many-branches
     magic_number = header[:2]
     file.seek(2)
     pnm_header = []
+    next_value = bytearray()
     while True:
         # We have all we need at length 3 for formats P2, P3, P5, P6
         if len(pnm_header) == 3:
-            break
+            if magic_number in [b"P2", b"P5"]:
+                from . import pgm
+
+                return pgm.load(
+                    file, magic_number, pnm_header, bitmap=bitmap, palette=palette
+                )
+
+            if magic_number == b"P3":
+                from . import ppm_ascii
+
+                return ppm_ascii.load(
+                    file, pnm_header[0], pnm_header[1], bitmap=bitmap, palette=palette
+                )
+
+            if magic_number == b"P6":
+                from . import ppm_binary
+
+                return ppm_binary.load(
+                    file, pnm_header[0], pnm_header[1], bitmap=bitmap, palette=palette
+                )
+
         if len(pnm_header) == 2 and magic_number in [b"P1", b"P4"]:
             bitmap = bitmap(pnm_header[0], pnm_header[1], 1)
             if palette:
@@ -66,46 +89,15 @@ def load(file, header, *, bitmap=None, palette=None):
             )
 
         next_byte = file.read(1)
-        if next_byte == b"#":
-            while True:
-                next_byte = file.read(1)
-                if not next_byte:
-                    raise RuntimeError("Unsupported image format")
-                if next_byte == b"\n":
-                    break
-        if next_byte.isdigit():
-            value = bytearray()
-            while True:
-                if not next_byte.isdigit():
-                    break
-                value += next_byte
-                next_byte = file.read(1)
-                if not next_byte:
-                    raise RuntimeError("Unsupported image format")
-
-            pnm_header.append(int("".join(["%c" % char for char in value])))
-            continue
-
-        if not next_byte:
-            raise RuntimeError("Unsupported image format")
-
-    if magic_number in [b"P2", b"P5"]:
-        from . import pgm
-
-        return pgm.load(file, magic_number, pnm_header, bitmap=bitmap, palette=palette)
-
-    if magic_number == b"P3":
-        from . import ppm_ascii
-
-        return ppm_ascii.load(
-            file, pnm_header[0], pnm_header[1], bitmap=bitmap, palette=palette
-        )
-
-    if magic_number == b"P6":
-        from . import ppm_binary
-
-        return ppm_binary.load(
-            file, pnm_header[0], pnm_header[1], bitmap=bitmap, palette=palette
-        )
-
-    raise RuntimeError("Unsupported image format {}".format(magic_number))
+        if next_byte == b"":
+            raise RuntimeError("Unsupported image format {}".format(magic_number))
+        if next_byte == b"#":  # comment found, seek until a newline or EOF is found
+            while file.read(1) not in [b"", b"\n"]:  # EOF or NL
+                pass
+        elif not next_byte.isdigit():  # boundary found in header data
+            if next_value:
+                # pull values until space is found
+                pnm_header.append(int("".join(["%c" % char for char in next_value])))
+                next_value = bytearray()  # reset the byte array
+        else:
+            next_value += next_byte  # push the digit into the byte array

adafruit_imageload/pnm/pgm/__init__.py

@@ -41,10 +41,12 @@ def load(file, magic_number, header, *, bitmap=None, palette=None):
 
     if magic_number == b"P2":  # To handle ascii PGM files.
         from . import ascii as pgm_ascii
+
         return pgm_ascii.load(file, width, height, bitmap=bitmap, palette=palette)
 
     if magic_number == b"P5":  # To handle binary PGM files.
         from . import binary
+
         return binary.load(file, width, height, bitmap=bitmap, palette=palette)
 
     raise NotImplementedError("Was not able to send image")

adafruit_imageload/pnm/ppm_ascii.py

@@ -36,55 +36,54 @@
 
 def load(file, width, height, bitmap=None, palette=None):
     """
-
     :param stream file: infile with the position set at start of data
     :param int width:
     :param int height:
     :param int max_colors: color space of file
     :param bitmap: displayio.Bitmap class
     :param palette: displayio.Palette class
-    :return:
+    :return tuple:
     """
     palette_colors = set()
     data_start = file.tell()
-    triplet = []
-    color = bytearray()
-    while True:  # scan for all colors present in the file
-        # read values from file, values can be string len 1-3 for values 0 - 255
-        this_byte = file.read(1)
-        if this_byte == b"":
-            break
-        if not this_byte.isdigit():  # completed one number
-            triplet.append(int("".join(["%c" % char for char in color])))
-            color = bytearray()
-            if len(triplet) == 3:
-                palette_colors.add(tuple(triplet))
-                triplet = []
-            continue
-        color += this_byte
+    for triplet in read_three_colors(file):
+        palette_colors.add(triplet)
+
     if palette:
         palette = palette(len(palette_colors))
         for counter, color in enumerate(palette_colors):
-            palette[counter] = bytes(color)
-
+            palette[counter] = color
     if bitmap:
         file.seek(data_start)
         bitmap = bitmap(width, height, len(palette_colors))
         palette_colors = list(palette_colors)
         for y in range(height):
             for x in range(width):
-                triplet = []
-                color = bytearray()
-                while True:
-                    this_byte = file.read(1)
+                for color in read_three_colors(file):
+                    bitmap[x, y] = palette_colors.index(color)
+                    break  # exit the inner generator
+    return bitmap, palette
 
-                    if not this_byte.isdigit():  # completed one number
-                        triplet.append(int("".join(["%c" % char for char in color])))
-                        color = bytearray()
-                        if len(triplet) == 3:  # completed one pixel
-                            bitmap[x, y] = palette_colors.index(tuple(triplet))
-                            break
-                        continue
-                    color += this_byte
 
-    return bitmap, palette
+def read_three_colors(file):
+    """
+    Generator to read integer values from file, in groups of three.
+    Each value can be len 1-3, for values 0 - 255, space padded.
+    :return tuple[int]:
+    """
+    triplet = []
+    color = bytearray()
+    while True:
+        this_byte = file.read(1)
+        if this_byte.isdigit():
+            color += this_byte
+        # not a digit means we completed one number (found a space separator or EOF)
+        elif color or (triplet and this_byte == b""):
+            triplet.append(int("".join(["%c" % char for char in color])))
+            color = bytearray()
+        if len(triplet) == 3:  # completed one pixel
+            yield bytes(tuple(triplet))
+            triplet = []
+        # short circuit must be after all other cases, so we yield the last pixel before returning
+        if this_byte == b"":
+            return

adafruit_imageload/pnm/ppm_binary.py

@@ -45,9 +45,8 @@ def load(file, width, height, bitmap=None, palette=None):
     for y in range(height):
         data_line = iter(bytes(file.read(line_size)))
         for red in data_line:
-            green = next(data_line)
-            blue = next(data_line)
-            palette_colors.add((red, green, blue))
+            # red, green, blue
+            palette_colors.add((red, next(data_line), next(data_line)))
 
     if palette:
         palette = palette(len(palette_colors))
@@ -61,9 +60,10 @@ def load(file, width, height, bitmap=None, palette=None):
             x = 0
             data_line = iter(bytes(file.read(line_size)))
             for red in data_line:
-                green = next(data_line)
-                blue = next(data_line)
-                bitmap[x, y] = palette_colors.index((red, green, blue))
+                # red, green, blue
+                bitmap[x, y] = palette_colors.index(
+                    (red, next(data_line), next(data_line))
+                )
                 x += 1
 
     return bitmap, palette

adafruit_imageload/tests/displayio_shared_bindings.py

@@ -75,6 +75,8 @@ def __setitem__(self, key: Union[tuple, int], value: int) -> None:
             raise RuntimeError(f"set value as int, not {type(value)}")
         if value > 255:
             raise ValueError(f"pixel value {value} too large")
+        if self.data.get(key):
+            raise ValueError(f"pixel {self._decode(key)}/{key} already set, cannot set again")
         self.data[key] = value
 
     def __getitem__(self, item: Union[tuple, int]) -> bytearray:
@@ -87,18 +89,22 @@ def __getitem__(self, item: Union[tuple, int]) -> bytearray:
         except KeyError:
             raise RuntimeError("no data at {} [{}]".format(self._decode(item), item))
 
-    def validate(self) -> None:
+    def validate(self, detect_empty_image=True) -> None:
         """
         method to to make sure all pixels allocated in the Bitmap
         were set with a value
         """
+        seen_colors = set()
         if not self.data:
             raise ValueError("no rows were set / no data in memory")
-        for i in range(self.height * self.width, 0):
-            try:
-                self.data[i]
-            except KeyError:
-                raise ValueError("missing data at {i}")
+        for y in range(self.height):
+            for x in range(self.width):
+                try:
+                    seen_colors.add(self[x,y])
+                except KeyError:
+                    raise ValueError(f"missing data at {x},{y}")
+        if detect_empty_image and len(seen_colors) < 2:
+            raise ValueError('image detected as only one color. set detect_empty_image=False to ignore')
 
     def __str__(self) -> str:
         """

adafruit_imageload/tests/test_ppm_load.py

@@ -27,8 +27,10 @@
 
 """
 import os
+from io import BytesIO
 from unittest import TestCase
 from .. import pnm
+from ..pnm.ppm_ascii import read_three_colors
 from .displayio_shared_bindings import Bitmap_C_Interface, Palette_C_Interface
 
 
@@ -45,12 +47,12 @@ def test_load_works_p3_ascii(self):
         with open(test_file, "rb") as file:
             bitmap, palette = pnm.load(
                 file, b"P3", bitmap=Bitmap_C_Interface, palette=Palette_C_Interface
-            )
+            )  # type: Bitmap_C_Interface, Palette_C_Interface
 
         self.assertTrue(isinstance(palette, Palette_C_Interface))
         self.assertEqual(6, palette.num_colors)
         palette.validate()
-        # self.fail(str(palette))
+        #self.fail(str(palette))
         self.assertTrue(isinstance(bitmap, Bitmap_C_Interface), bitmap)
         self.assertEqual(6, bitmap.colors)
         self.assertEqual(16, bitmap.width)
@@ -69,11 +71,22 @@ def test_load_works_p6_binary(self):
         with open(test_file, "rb") as file:
             bitmap, palette = pnm.load(
                 file, b"P6", bitmap=Bitmap_C_Interface, palette=Palette_C_Interface
-            )
+            )  # type: Bitmap_C_Interface, Palette_C_Interface
         self.assertEqual(6, palette.num_colors)
         palette.validate()
         self.assertTrue(isinstance(bitmap, Bitmap_C_Interface), bitmap)
         self.assertEqual(6, bitmap.colors)
         self.assertEqual(16, bitmap.width)
         self.assertEqual(16, bitmap.height)
         bitmap.validate()
+
+    def test_load_three_colors_tail(self):
+        buffer = BytesIO(b'211 222 233')
+        for i in read_three_colors(buffer):
+            self.assertEqual(b'\xd3\xde\xe9', i)
+
+    def test_load_three_colors_middle(self):
+        buffer = BytesIO(b'0 128 255 45 55 25')
+        for i in iter(read_three_colors(buffer)):
+            self.assertEqual(b'\x00\x80\xff', i)
+            break