diff --git a/packtools/sps/formats/pdf/pipeline/docx.py b/packtools/sps/formats/pdf/pipeline/docx.py index 9c104f58a..1ec72bb8c 100644 --- a/packtools/sps/formats/pdf/pipeline/docx.py +++ b/packtools/sps/formats/pdf/pipeline/docx.py @@ -1,3 +1,5 @@ +import statistics + from packtools.sps.formats.pdf import enum as pdf_enum from packtools.sps.formats.pdf.pipeline import xml as xml_pipe from packtools.sps.formats.pdf.renderer import docx as docx_renderer @@ -548,8 +550,39 @@ def _figure_layout(docx, fig): return layout +def _flag_dpi_outliers(docx, figures, outlier_ratio=2.0): + """ + Compare embedded DPI across a batch of sibling figures (e.g. all the + graphs in one section) and flag figures whose own DPI is wildly off from + the group's median, so decide_figure_layout can be given a corrected + value instead of trusting a possibly-wrong per-image tag. Figures with + an explicit 'layout' override are left untouched; outliers get + 'layout_dpi_override' set to the group median. + """ + probed = [] + for fig in figures: + if not isinstance(fig, dict) or fig.get('layout'): + continue + probe = docx_renderer.figure.probe_image_dpi(docx, fig) + if probe is not None: + probed.append((fig, probe[1])) + + if len(probed) < 2: + return + + median_dpi = statistics.median(dpi for _, dpi in probed) + if median_dpi <= 0: + return + + for fig, dpi in probed: + ratio = dpi / median_dpi + if ratio >= outlier_ratio or ratio <= 1 / outlier_ratio: + fig['layout_dpi_override'] = median_dpi + + def _render_figures(docx, figures): """Render figures, switching to single column when the layout requires it.""" + _flag_dpi_outliers(docx, figures) for fig in figures: layout = _figure_layout(docx, fig) diff --git a/packtools/sps/formats/pdf/pipeline/xml.py b/packtools/sps/formats/pdf/pipeline/xml.py index 92d54758c..d5f67df68 100644 --- a/packtools/sps/formats/pdf/pipeline/xml.py +++ b/packtools/sps/formats/pdf/pipeline/xml.py @@ -423,11 +423,14 @@ def _get_href_from_node(node): caption_texts.append(txt) caption = ' '.join([c for c in caption_texts if c]) - # graphic may be direct child or inside + # graphic may be a direct child, or offered as several representations + # inside - only match the direct child here so the latter + # case falls through to the ranking logic below instead of grabbing the + # first in document order. href = None alt_text = None - graphic = fig_node.find('.//graphic') + graphic = fig_node.find('graphic') if graphic is not None: href = _get_href_from_node(graphic) alt_text = graphic.get('alt') or graphic.get('alt-text') diff --git a/packtools/sps/formats/pdf/renderer/docx/figure.py b/packtools/sps/formats/pdf/renderer/docx/figure.py index 4139d5cc4..6d228c2f2 100644 --- a/packtools/sps/formats/pdf/renderer/docx/figure.py +++ b/packtools/sps/formats/pdf/renderer/docx/figure.py @@ -38,7 +38,7 @@ def decide_figure_layout(docx, figure_data, page_attributes=pdf_enum.PAGE_ATTRIB Decide whether a figure should occupy the full page width (double-column-layout) or a single column (single-column-layout). Heuristic: - - Compute the natural width of the image in centimeters using Pillow and its DPI metadata (tries dpi, jfif_density; defaults to 150 DPI when missing). + - Compute the natural width of the image in centimeters using Pillow and its DPI metadata (tries dpi, jfif_density; defaults to 300 DPI when missing). - Compute the available content width (page width minus margins) and the single-column width ((content - column_spacing)/2). - If natural image width >= threshold * single-column width, return 'double-column-layout'; otherwise 'single-column-layout'. @@ -82,40 +82,69 @@ def decide_figure_layout(docx, figure_data, page_attributes=pdf_enum.PAGE_ATTRIB column_spacing_cm = Cm(column_spacing_twips / 567.0) single_col_width = (content_width - column_spacing_cm) / 2 - # Resolve image path (local or download) + probe = probe_image_dpi(docx, figure_data) + if probe is None: + return pdf_enum.SINGLE_COLUMN_PAGE_LABEL + px_w, dpi = probe + + # A caller may have compared this image's DPI against its siblings and + # supplied a more trustworthy value here - see 'layout_dpi_override'. + if isinstance(figure_data, dict) and figure_data.get('layout_dpi_override'): + try: + dpi = float(figure_data['layout_dpi_override']) + except (TypeError, ValueError): + pass + + # width in inches then to Cm + width_in_cm = (px_w / max(1.0, dpi)) * 2.54 + + # single_col_width degraded from a Cm object to a raw EMU number in the + # subtraction/division above (python-docx's Length has no operator + # overloads that preserve units) - convert back to cm so this compares + # against width_in_cm in the same unit, instead of cm against EMU. + single_col_width_cm = single_col_width / Cm(1) + + # If the image is wider than a single column by the threshold factor, prefer a single-column-layout (full width). Otherwise, keep double-column-layout. + return pdf_enum.SINGLE_COLUMN_PAGE_LABEL if width_in_cm >= float(threshold) * single_col_width_cm else pdf_enum.DOUBLE_COLUMN_PAGE_LABEL + + +def probe_image_dpi(docx, figure_data): + """ + Resolve a figure's image and return its (pixel_width, dpi), or None if + the image can't be opened. + """ + try: + from PIL import Image + except Exception: + return None + context = _get_docx_context(docx) href = figure_data.get('href') if isinstance(figure_data, dict) else None img_path = _resolve_image_path(href, context) - if not img_path: - return pdf_enum.SINGLE_COLUMN_PAGE_LABEL + if not img_path or not os.path.exists(img_path): + return None - # Open image and compute its natural width in Cm using DPI (default 72 DPI) - if not os.path.exists(img_path): - return pdf_enum.SINGLE_COLUMN_PAGE_LABEL try: with Image.open(img_path) as im: - px_w = im.width - dpi = _infer_image_dpi(im) - # width in inches then to Cm - width_in_cm = (px_w / max(1.0, dpi)) * 2.54 + return im.width, _infer_image_dpi(im) except Exception: - return pdf_enum.SINGLE_COLUMN_PAGE_LABEL - - # If the image is wider than a single column by the threshold factor, prefer a single-column-layout (full width). Otherwise, keep double-column-layout. - return pdf_enum.SINGLE_COLUMN_PAGE_LABEL if width_in_cm >= float(threshold) * float(single_col_width) else pdf_enum.DOUBLE_COLUMN_PAGE_LABEL + return None # ----------------- # Private helpers # ----------------- +_NO_METADATA_DPI_FALLBACK = 300.0 + + def _infer_image_dpi(im) -> float: """Infer the horizontal DPI from a PIL Image, considering multiple metadata sources. Priority: - im.info['dpi']: tuple or number - im.info['jfif_unit'] and im.info['jfif_density'] (unit 1=inches, 2=cm) - Fallback: 96 dpi + Fallback: _NO_METADATA_DPI_FALLBACK. """ try: info = getattr(im, 'info', {}) or {} @@ -138,10 +167,10 @@ def _infer_image_dpi(im) -> float: if unit == 2: # per cm return float(density[0]) * 2.54 - return 96.0 + return _NO_METADATA_DPI_FALLBACK except Exception: - return 96.0 + return _NO_METADATA_DPI_FALLBACK def _add_paragraph_with_formatting(docx, text, style_name='SCL Paragraph'): """Minimal helper to add a paragraph with an optional style, avoiding circular imports.""" diff --git a/tests/sps/formats/pdf/pipeline/test_docx.py b/tests/sps/formats/pdf/pipeline/test_docx.py index 0755081e4..afbc90192 100644 --- a/tests/sps/formats/pdf/pipeline/test_docx.py +++ b/tests/sps/formats/pdf/pipeline/test_docx.py @@ -1,4 +1,5 @@ import unittest +from unittest.mock import patch from packtools.sps.formats.pdf.pipeline import docx as docx_pipe @@ -91,3 +92,57 @@ class TestDocxAcknowledgmentsPipe(unittest.TestCase): class TestDocxSupplementaryMaterialPipe(unittest.TestCase): # TODO ... + + +class TestFlagDpiOutliers(unittest.TestCase): + """Tests for _flag_dpi_outliers: flags a figure's DPI as untrustworthy + when it diverges from its sibling figures' median DPI, in the same + batch, by more than outlier_ratio.""" + + def test_flags_dpi_outlier_against_siblings(self): + figures = [{'href': 'graph1.tif'}, {'href': 'graph2.tif'}, {'href': 'graph3.tif'}] + probes = {'graph1.tif': (612, 72.0), 'graph2.tif': (800, 300.0), 'graph3.tif': (700, 300.0)} + with patch( + 'packtools.sps.formats.pdf.renderer.docx.figure.probe_image_dpi', + side_effect=lambda docx, fig: probes[fig['href']], + ): + docx_pipe._flag_dpi_outliers(docx=None, figures=figures) + + self.assertEqual(figures[0]['layout_dpi_override'], 300.0) + self.assertNotIn('layout_dpi_override', figures[1]) + self.assertNotIn('layout_dpi_override', figures[2]) + + def test_does_not_flag_similar_dpis(self): + figures = [{'href': 'a.tif'}, {'href': 'b.tif'}] + probes = {'a.tif': (700, 280.0), 'b.tif': (700, 300.0)} + with patch( + 'packtools.sps.formats.pdf.renderer.docx.figure.probe_image_dpi', + side_effect=lambda docx, fig: probes[fig['href']], + ): + docx_pipe._flag_dpi_outliers(docx=None, figures=figures) + + self.assertNotIn('layout_dpi_override', figures[0]) + self.assertNotIn('layout_dpi_override', figures[1]) + + def test_skips_figures_with_explicit_layout(self): + # 'a.tif' already has a resolved layout and must not be probed; only + # 'b.tif' should be. With a single figure left to probe there aren't + # enough siblings to compare against, so no override is set either. + figures = [{'href': 'a.tif', 'layout': 'single-column-layout'}, {'href': 'b.tif'}] + with patch( + 'packtools.sps.formats.pdf.renderer.docx.figure.probe_image_dpi', + return_value=(700, 300.0), + ) as mock_probe: + docx_pipe._flag_dpi_outliers(docx=None, figures=figures) + mock_probe.assert_called_once_with(None, figures[1]) + self.assertNotIn('layout_dpi_override', figures[0]) + self.assertNotIn('layout_dpi_override', figures[1]) + + def test_single_figure_is_never_flagged(self): + figures = [{'href': 'a.tif'}] + with patch( + 'packtools.sps.formats.pdf.renderer.docx.figure.probe_image_dpi', + return_value=(612, 72.0), + ): + docx_pipe._flag_dpi_outliers(docx=None, figures=figures) + self.assertNotIn('layout_dpi_override', figures[0]) diff --git a/tests/sps/formats/pdf/pipeline/test_xml.py b/tests/sps/formats/pdf/pipeline/test_xml.py index f3b428a93..d4fc1c647 100644 --- a/tests/sps/formats/pdf/pipeline/test_xml.py +++ b/tests/sps/formats/pdf/pipeline/test_xml.py @@ -1238,7 +1238,48 @@ def test_extract_trans_abstract_data_custom_namespace(self): } ] result = xml_pipe.extract_trans_abstract_data( - xml, + xml, namespaces={'xml': 'http://custom.namespace'} ) self.assertEqual(result, expected) + + +class TestExtractFigureData(unittest.TestCase): + """Tests for extract_figure_data's graphic href resolution, including + the ranking logic used when a figure only offers .""" + + def test_direct_graphic_child_is_used_as_is(self): + xml = etree.fromstring( + '' + '' + '' + '' + ) + result = xml_pipe.extract_figure_data(xml) + self.assertEqual(result['href'], 'figure1.jpg') + + def test_alternatives_prefers_scielo_web_over_raw_tif(self): + xml = etree.fromstring( + '' + '' + '' + '' + '' + '' + '' + '' + ) + result = xml_pipe.extract_figure_data(xml) + self.assertEqual(result['href'], 'web.png') + + def test_alternatives_falls_back_to_tif_when_no_better_option(self): + xml = etree.fromstring( + '' + '' + '' + '' + '' + '' + ) + result = xml_pipe.extract_figure_data(xml) + self.assertEqual(result['href'], 'raw.tif') diff --git a/tests/sps/formats/pdf/renderer/__init__.py b/tests/sps/formats/pdf/renderer/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/sps/formats/pdf/renderer/docx/__init__.py b/tests/sps/formats/pdf/renderer/docx/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/sps/formats/pdf/renderer/docx/test_figure.py b/tests/sps/formats/pdf/renderer/docx/test_figure.py new file mode 100644 index 000000000..c8a00c07d --- /dev/null +++ b/tests/sps/formats/pdf/renderer/docx/test_figure.py @@ -0,0 +1,77 @@ +import os +import tempfile +import unittest + +from docx import Document +from PIL import Image + +from packtools.sps.formats.pdf.renderer.docx.figure import decide_figure_layout +from packtools.sps.formats.pdf import enum as pdf_enum + + +class TestDecideFigureLayoutUnits(unittest.TestCase): + """ + Regression test for a units bug: single_col_width degrades from a Cm + object to a raw EMU number under python-docx's Length arithmetic (it has + no operator overloads that preserve units), so comparing it directly + against width_in_cm (a real centimeter float) compared cm against EMU - + a ~360000x scale mismatch that made the full-width branch unreachable + for any real image, regardless of size. + """ + + def setUp(self): + self.tmpdir = tempfile.TemporaryDirectory() + self.addCleanup(self.tmpdir.cleanup) + + def _make_png(self, px_width, px_height=200, dpi=96): + path = os.path.join(self.tmpdir.name, f"img_{px_width}.png") + Image.new("RGB", (px_width, px_height), color="white").save( + path, format="PNG", dpi=(dpi, dpi) + ) + return path + + def test_wide_image_gets_full_width_layout(self): + # ~18.5cm at 96 DPI - wider than a single column (~8.2cm) by far more + # than the 1.1 threshold, on an A4/2-column default page. + img_path = self._make_png(px_width=698) + docx = Document() + fig = {"href": img_path, "label": "Figure 1"} + self.assertEqual( + decide_figure_layout(docx, fig), pdf_enum.SINGLE_COLUMN_PAGE_LABEL + ) + + def test_narrow_image_stays_within_column(self): + # ~2.6cm at 96 DPI - well under the single-column width. + img_path = self._make_png(px_width=100) + docx = Document() + fig = {"href": img_path, "label": "Figure 1"} + self.assertEqual( + decide_figure_layout(docx, fig), pdf_enum.DOUBLE_COLUMN_PAGE_LABEL + ) + + def test_untagged_image_uses_print_resolution_fallback(self): + # An image with no DPI tag at all must fall back to print + # resolution, not screen resolution. + img_path = self._make_png(px_width=700) + with Image.open(img_path) as im: + im.info.pop("dpi", None) + im.save(img_path) # re-save without the dpi tag + docx = Document() + fig = {"href": img_path, "label": "Graph 1"} + self.assertEqual( + decide_figure_layout(docx, fig), pdf_enum.DOUBLE_COLUMN_PAGE_LABEL + ) + + def test_layout_dpi_override_replaces_embedded_dpi(self): + # 'layout_dpi_override' must take precedence over the image's own + # embedded DPI. + img_path = self._make_png(px_width=612, dpi=72) + docx = Document() + fig = {"href": img_path, "label": "Graph 1", "layout_dpi_override": 300.0} + self.assertEqual( + decide_figure_layout(docx, fig), pdf_enum.DOUBLE_COLUMN_PAGE_LABEL + ) + + +if __name__ == "__main__": + unittest.main()