refac: introduce BaseImageHeader base class

Steve Canny · Steve Canny · commit d89eac6ef318 · 2014-03-01T01:35:29.000-08:00
diff --git a/docx/image/image.py b/docx/image/image.py
@@ -17,13 +17,11 @@ class Image(object):
     Graphical image stream such as JPEG, PNG, or GIF with properties and
     methods required by ImagePart.
     """
-    def __init__(self, blob, filename, px_width, px_height, attrs):
+    def __init__(self, blob, filename, image_header):
         super(Image, self).__init__()
         self._blob = blob
         self._filename = filename
-        self._px_width = px_width
-        self._px_height = px_height
-        self._attrs = attrs
+        self._image_header = image_header
 
     @classmethod
     def from_file(cls, image_descriptor):
@@ -64,7 +62,54 @@ def _from_stream(cls, stream, blob, filename=None):
         Return an instance of the |Image| subclass corresponding to the
         format of the image in *stream*.
         """
-        # import at execution time to avoid circular import
-        from docx.image import image_cls_that_can_parse
-        ImageSubclass = image_cls_that_can_parse(stream)
-        return ImageSubclass.from_stream(stream, blob, filename)
+        image_header = _ImageHeaderFactory(stream)
+        return cls(blob, filename, image_header)
+
+
+def _ImageHeaderFactory(stream):
+    """
+    Return a |BaseImageHeader| subclass instance that knows how to parse the
+    headers of the image in *stream*.
+    """
+    raise NotImplementedError
+
+
+class BaseImageHeader(object):
+    """
+    Base class for image header subclasses like |Jpeg| and |Tiff|.
+    """
+    def __init__(self, px_width, px_height, horz_dpi, vert_dpi):
+        self._px_width = px_width
+        self._px_height = px_height
+        self._horz_dpi = horz_dpi
+        self._vert_dpi = vert_dpi
+
+    @property
+    def px_width(self):
+        """
+        The horizontal pixel dimension of the image
+        """
+        return self._px_width
+
+    @property
+    def px_height(self):
+        """
+        The vertical pixel dimension of the image
+        """
+        return self._px_height
+
+    @property
+    def horz_dpi(self):
+        """
+        Integer dots per inch for the width of this image. Defaults to 72
+        when not present in the file, as is often the case.
+        """
+        return self._horz_dpi
+
+    @property
+    def vert_dpi(self):
+        """
+        Integer dots per inch for the height of this image. Defaults to 72
+        when not present in the file, as is often the case.
+        """
+        return self._vert_dpi
diff --git a/docx/image/jpeg.py b/docx/image/jpeg.py
@@ -10,19 +10,14 @@
 from ..compat import BytesIO
 from .constants import JPEG_MARKER_CODE, MIME_TYPE
 from .helpers import BIG_ENDIAN, StreamReader
-from .image import Image
+from .image import BaseImageHeader
 from .tiff import Tiff
 
 
-class Jpeg(Image):
+class Jpeg(BaseImageHeader):
     """
     Base class for JFIF and EXIF subclasses.
     """
-    def __init__(self, blob, filename, cx, cy, horz_dpi, vert_dpi):
-        super(Jpeg, self).__init__(blob, filename, cx, cy, attrs={})
-        self._horz_dpi = horz_dpi
-        self._vert_dpi = vert_dpi
-
     @property
     def content_type(self):
         """
@@ -31,22 +26,6 @@ def content_type(self):
         """
         return MIME_TYPE.JPEG
 
-    @property
-    def horz_dpi(self):
-        """
-        Integer dots per inch for the width of this image. Defaults to 72
-        when not present in the file, as is often the case.
-        """
-        return self._horz_dpi
-
-    @property
-    def vert_dpi(self):
-        """
-        Integer dots per inch for the height of this image. Defaults to 72
-        when not present in the file, as is often the case.
-        """
-        return self._vert_dpi
-
 
 class Exif(Jpeg):
     """
@@ -66,7 +45,7 @@ def from_stream(cls, stream, blob, filename):
         horz_dpi = markers.app1.horz_dpi
         vert_dpi = markers.app1.vert_dpi
 
-        return cls(blob, filename, px_width, px_height, horz_dpi, vert_dpi)
+        return cls(px_width, px_height, horz_dpi, vert_dpi)
 
 
 class Jfif(Jpeg):
@@ -80,10 +59,13 @@ def from_stream(cls, stream, blob, filename):
         in *stream*.
         """
         markers = _JfifMarkers.from_stream(stream)
-        sof, app0 = markers.sof, markers.app0
-        cx, cy = sof.px_width, sof.px_height
-        horz_dpi, vert_dpi = app0.horz_dpi, app0.vert_dpi
-        return cls(blob, filename, cx, cy, horz_dpi, vert_dpi)
+
+        px_width = markers.sof.px_width
+        px_height = markers.sof.px_height
+        horz_dpi = markers.app0.horz_dpi
+        vert_dpi = markers.app0.vert_dpi
+
+        return cls(px_width, px_height, horz_dpi, vert_dpi)
 
 
 class _JfifMarkers(object):
diff --git a/docx/image/png.py b/docx/image/png.py
@@ -5,20 +5,21 @@
 from .constants import MIME_TYPE, TAG
 from .exceptions import InvalidImageStreamError
 from .helpers import StreamReader
-from .image import Image
+from .image import BaseImageHeader
 
 
 _CHUNK_TYPE_IHDR = 'IHDR'
 _CHUNK_TYPE_pHYs = 'pHYs'
 _CHUNK_TYPE_IEND = 'IEND'
 
 
-class Png(Image):
+class Png(BaseImageHeader):
     """
     Image header parser for PNG images
     """
     def __init__(self, blob, filename, cx, cy, attrs):
-        super(Png, self).__init__(blob, filename, cx, cy, attrs)
+        super(Png, self).__init__(cx, cy, None, None)
+        self._attrs = attrs
 
     @property
     def content_type(self):
diff --git a/docx/image/tiff.py b/docx/image/tiff.py
@@ -4,19 +4,14 @@
 
 from .constants import MIME_TYPE, TIFF_FLD, TIFF_TAG
 from .helpers import BIG_ENDIAN, LITTLE_ENDIAN, StreamReader
-from .image import Image
+from .image import BaseImageHeader
 
 
-class Tiff(Image):
+class Tiff(BaseImageHeader):
     """
     Image header parser for TIFF images. Handles both big and little endian
     byte ordering.
     """
-    def __init__(self, blob, filename, cx, cy, horz_dpi, vert_dpi):
-        super(Tiff, self).__init__(blob, filename, cx, cy, {})
-        self._horz_dpi = horz_dpi
-        self._vert_dpi = vert_dpi
-
     @property
     def content_type(self):
         """
@@ -32,25 +27,13 @@ def from_stream(cls, stream, blob, filename):
         in *stream*.
         """
         parser = _TiffParser.parse(stream)
+
         px_width = parser.px_width
         px_height = parser.px_height
         horz_dpi = parser.horz_dpi
         vert_dpi = parser.vert_dpi
-        return cls(blob, filename, px_width, px_height, horz_dpi, vert_dpi)
 
-    @property
-    def horz_dpi(self):
-        """
-        The intended print density of the image's pixels along the x-axis
-        """
-        return self._horz_dpi
-
-    @property
-    def vert_dpi(self):
-        """
-        The intended print density of the image's pixels along the y-axis
-        """
-        return self._vert_dpi
+        return cls(px_width, px_height, horz_dpi, vert_dpi)
 
 
 class _TiffParser(object):
diff --git a/features/img-characterize-image.feature b/features/img-characterize-image.feature
@@ -3,6 +3,7 @@ Feature: Characterize an image file
   As a programmer using the advanced python-docx API
   I need a way to determine the image content type and size
 
+  @wip
   Scenario Outline: Characterize an image file
     Given the image file '<filename>'
      When I construct an image using the image path
diff --git a/tests/image/test_image.py b/tests/image/test_image.py
@@ -13,14 +13,14 @@
 from docx.image.bmp import Bmp
 from docx.image.exceptions import UnrecognizedImageError
 from docx.image.gif import Gif
-from docx.image.image import Image
+from docx.image.image import BaseImageHeader, Image
 from docx.image.jpeg import Exif, Jfif
 from docx.image.png import Png
 from docx.image.tiff import Tiff
 from docx.opc.constants import CONTENT_TYPE as CT
 
 from ..unitutil import (
-    function_mock, class_mock, instance_mock, loose_mock, method_mock,
+    function_mock, class_mock, initializer_mock, instance_mock, method_mock,
     test_file
 )
 
@@ -75,14 +75,16 @@ def it_can_construct_from_an_image_file_like(self, from_filelike_fixture):
         assert image is image_
 
     def it_can_construct_from_an_image_stream(self, from_stream_fixture):
-        (stream_, blob_, filename_, image_, image_cls_that_can_parse_,
-         image_cls_) = from_stream_fixture
+        # fixture ----------------------
+        stream_, blob_, filename_ = from_stream_fixture[:3]
+        _ImageHeaderFactory_, image_header_ = from_stream_fixture[3:5]
+        Image__init_ = from_stream_fixture[5]
+        # exercise ---------------------
         image = Image._from_stream(stream_, blob_, filename_)
-        image_cls_that_can_parse_.assert_called_once_with(stream_)
-        image_cls_.from_stream.assert_called_once_with(
-            stream_, blob_, filename_
-        )
-        assert image is image_
+        # verify -----------------------
+        _ImageHeaderFactory_.assert_called_once_with(stream_)
+        Image__init_.assert_called_once_with(blob_, filename_, image_header_)
+        assert isinstance(image, Image)
 
     # fixtures -------------------------------------------------------
 
@@ -118,11 +120,11 @@ def from_path_fixture(self, _from_stream_, BytesIO_, stream_, image_):
 
     @pytest.fixture
     def from_stream_fixture(
-            self, stream_, blob_, filename_, image_,
-            image_cls_that_can_parse_, image_cls_):
+            self, stream_, blob_, filename_, _ImageHeaderFactory_,
+            image_header_, Image__init_):
         return (
-            stream_, blob_, filename_, image_, image_cls_that_can_parse_,
-            image_cls_
+            stream_, blob_, filename_, _ImageHeaderFactory_, image_header_,
+            Image__init_
         )
 
     @pytest.fixture
@@ -136,23 +138,40 @@ def image_(self, request):
         return instance_mock(request, Image)
 
     @pytest.fixture
-    def image_cls_(self, request, image_):
-        image_cls_ = loose_mock(request)
-        image_cls_.from_stream.return_value = image_
-        return image_cls_
-
-    @pytest.fixture
-    def image_cls_that_can_parse_(self, request, image_cls_):
+    def _ImageHeaderFactory_(self, request, image_header_):
         return function_mock(
-            request, 'docx.image.image_cls_that_can_parse',
-            return_value=image_cls_
+            request, 'docx.image.image._ImageHeaderFactory',
+            return_value=image_header_
         )
 
+    @pytest.fixture
+    def image_header_(self, request):
+        return instance_mock(request, BaseImageHeader)
+
+    @pytest.fixture
+    def Image__init_(self, request):
+        return initializer_mock(request, Image)
+
     @pytest.fixture
     def stream_(self, request):
         return instance_mock(request, BytesIO)
 
 
+class DescribeBaseImageHeader(object):
+
+    def it_knows_the_image_dimensions(self):
+        px_width, px_height = 42, 24
+        image_header = BaseImageHeader(px_width, px_height, None, None)
+        assert image_header.px_width == px_width
+        assert image_header.px_height == px_height
+
+    def it_knows_the_horz_and_vert_dpi_of_the_image(self):
+        horz_dpi, vert_dpi = 42, 24
+        image_header = BaseImageHeader(None, None, horz_dpi, vert_dpi)
+        assert image_header.horz_dpi == horz_dpi
+        assert image_header.vert_dpi == vert_dpi
+
+
 class DescribeImage_OLD(object):
 
     def it_can_construct_from_an_image_path(self):
diff --git a/tests/image/test_jpeg.py b/tests/image/test_jpeg.py
@@ -27,7 +27,7 @@
 class DescribeJpeg(object):
 
     def it_knows_its_content_type(self):
-        jpeg = Jpeg(None, None, None, None, None, None)
+        jpeg = Jpeg(None, None, None, None)
         assert jpeg.content_type == MIME_TYPE.JPEG
 
 
diff --git a/tests/image/test_tiff.py b/tests/image/test_tiff.py
@@ -32,20 +32,14 @@ def it_can_construct_from_a_tiff_stream(self, from_stream_fixture):
         tiff = Tiff.from_stream(stream_, blob_, filename_)
         _TiffParser_.parse.assert_called_once_with(stream_)
         Tiff__init_.assert_called_once_with(
-            blob_, filename_, px_width, px_height, horz_dpi, vert_dpi
+            px_width, px_height, horz_dpi, vert_dpi
         )
         assert isinstance(tiff, Tiff)
 
     def it_knows_its_content_type(self):
-        tiff = Tiff(None, None, None, None, None, None)
+        tiff = Tiff(None, None, None, None)
         assert tiff.content_type == MIME_TYPE.TIFF
 
-    def it_knows_the_horz_and_vert_dpi_of_the_tiff_image(self):
-        horz_dpi, vert_dpi = 42, 24
-        tiff = Tiff(None, None, None, None, horz_dpi, vert_dpi)
-        assert tiff.horz_dpi == horz_dpi
-        assert tiff.vert_dpi == vert_dpi
-
     # fixtures -------------------------------------------------------
 
     @pytest.fixture
