From 17a5b8b43c6afe4455bb9baa436b6046186e5cd2 Mon Sep 17 00:00:00 2001 From: "James R. Barlow" Date: Mon, 13 Jun 2022 01:30:15 -0700 Subject: [PATCH] Refactor reporting of optimization failures --- src/ocrmypdf/_pipeline.py | 4 ++-- src/ocrmypdf/_sync.py | 18 ++++++++++----- src/ocrmypdf/_validation.py | 28 ++++++++++-------------- src/ocrmypdf/builtin_plugins/optimize.py | 21 +++++++++++++++--- src/ocrmypdf/optimize.py | 2 +- src/ocrmypdf/pluginspec.py | 16 ++++++++++++-- tests/test_validation.py | 16 +++++--------- 7 files changed, 64 insertions(+), 41 deletions(-) diff --git a/src/ocrmypdf/_pipeline.py b/src/ocrmypdf/_pipeline.py index 15efde3a..de97c5f8 100644 --- a/src/ocrmypdf/_pipeline.py +++ b/src/ocrmypdf/_pipeline.py @@ -833,7 +833,7 @@ def metadata_fixup(working_file: Path, context: PdfContext): def optimize_pdf(input_file: Path, context: PdfContext, executor: Executor): output_file = context.get_path('optimize.pdf') - output_pdf = context.plugin_manager.hook.optimize_pdf( + output_pdf, messages = context.plugin_manager.hook.optimize_pdf( input_pdf=input_file, output_pdf=output_file, context=context, @@ -848,7 +848,7 @@ def optimize_pdf(input_file: Path, context: PdfContext, executor: Executor): savings = 1 - output_size / input_size log.info(f"Optimize ratio: {ratio:.2f} savings: {(savings):.1%}") - return output_pdf + return output_pdf, messages def enumerate_compress_ranges(iterable): diff --git a/src/ocrmypdf/_sync.py b/src/ocrmypdf/_sync.py index 93213434..f3a54195 100644 --- a/src/ocrmypdf/_sync.py +++ b/src/ocrmypdf/_sync.py @@ -18,7 +18,7 @@ from concurrent.futures.thread import BrokenThreadPool from functools import partial from pathlib import Path from tempfile import mkdtemp -from typing import List, NamedTuple, Optional, Tuple, cast +from typing import List, NamedTuple, Optional, Sequence, Tuple, cast import PIL @@ -230,7 +230,9 @@ def exec_page_sync(page_context: PageContext) -> PageResult: ) -def post_process(pdf_file: Path, context: PdfContext, executor: Executor) -> Path: +def post_process( + pdf_file: Path, context: PdfContext, executor: Executor +) -> Tuple[Path, Sequence[str]]: pdf_out = pdf_file if context.options.output_type.startswith('pdfa'): ps_stub_out = generate_postscript_stub(context) @@ -248,7 +250,7 @@ def worker_init(max_pixels: int) -> None: pikepdf_enable_mmap() -def exec_concurrent(context: PdfContext, executor: Executor) -> None: +def exec_concurrent(context: PdfContext, executor: Executor) -> Sequence[str]: """Execute the pipeline concurrently""" # Run exec_page_sync on every page context @@ -300,13 +302,15 @@ def exec_concurrent(context: PdfContext, executor: Executor) -> None: # Merge layers to one single pdf pdf = ocrgraft.finalize() + messages: List[str] = [] if options.output_type != 'none': # PDF/A and metadata log.info("Postprocessing...") - pdf = post_process(pdf, context, executor) + pdf, messages = post_process(pdf, context, executor) # Copy PDF file to destination copy_final(pdf, options.output_file, context) + return messages def configure_debug_logging( @@ -386,7 +390,7 @@ def run_pipeline( validate_pdfinfo_options(context) # Execute the pipeline - exec_concurrent(context, executor) + optimize_messages = exec_concurrent(context, executor) if options.output_file == '-': log.info("Output sent to stdout") @@ -412,7 +416,9 @@ def run_pipeline( if not check_pdf(options.output_file): log.warning('Output file: The generated PDF is INVALID') return ExitCode.invalid_output_pdf - report_output_file_size(options, start_input_file, options.output_file) + report_output_file_size( + options, start_input_file, options.output_file, optimize_messages + ) except (KeyboardInterrupt if not api else NeverRaise): if options.verbose >= 1: diff --git a/src/ocrmypdf/_validation.py b/src/ocrmypdf/_validation.py index 275dad9a..e7b62119 100644 --- a/src/ocrmypdf/_validation.py +++ b/src/ocrmypdf/_validation.py @@ -14,12 +14,12 @@ import sys import unicodedata from pathlib import Path from shutil import copyfileobj -from typing import List, Set, Tuple +from typing import List, Optional, Sequence, Set, Tuple import pikepdf import PIL -from ocrmypdf._exec import jbig2enc, pngquant, unpaper +from ocrmypdf._exec import unpaper from ocrmypdf.exceptions import ( BadArgsError, InputFileError, @@ -294,8 +294,15 @@ def check_requested_output_file(options): def report_output_file_size( - options, input_file, output_file, file_overhead=4000, page_overhead=3000 + options, + input_file: Path, + output_file: Path, + optimize_messages: Optional[Sequence[str]] = None, + file_overhead: int = 4000, + page_overhead: int = 3000, ): + if optimize_messages is None: + optimize_messages = [] try: output_size = Path(output_file).stat().st_size input_size = Path(input_file).stat().st_size @@ -324,19 +331,8 @@ def report_output_file_size( f"The argument --{arg.replace('_', '-')} was issued, causing transcoding." ) - if hasattr(options, 'optimize') and options.optimize == 0: - reasons.append("Optimization was disabled.") - else: - image_optimizers = { - 'jbig2': jbig2enc.available(), - 'pngquant': pngquant.available(), - } - for name, available in image_optimizers.items(): - if not available: - reasons.append( - f"The optional dependency '{name}' was not found, so some image " - f"optimizations could not be attempted." - ) + reasons.extend(optimize_messages) + if options.output_type.startswith('pdfa'): reasons.append("PDF/A conversion was enabled. (Try `--output-type pdf`.)") if options.plugins: diff --git a/src/ocrmypdf/builtin_plugins/optimize.py b/src/ocrmypdf/builtin_plugins/optimize.py index fed0a997..f46a4b76 100644 --- a/src/ocrmypdf/builtin_plugins/optimize.py +++ b/src/ocrmypdf/builtin_plugins/optimize.py @@ -10,6 +10,7 @@ import argparse import logging from pathlib import Path +from typing import Sequence, Tuple from ocrmypdf import PdfContext, hookimpl from ocrmypdf._concurrent import Executor @@ -130,13 +131,27 @@ def optimize_pdf( context: PdfContext, executor: Executor, linearize: bool, -) -> Path: +) -> Tuple[Path, Sequence[str]]: save_settings = dict( linearize=linearize, **get_pdf_save_settings(context.options.output_type), ) - optimize(input_pdf, output_pdf, context, save_settings, executor) - return output_pdf + result_path = optimize(input_pdf, output_pdf, context, save_settings, executor) + messages = [] + if context.options.optimize == 0: + messages.append("Optimization was disabled.") + else: + image_optimizers = { + 'jbig2': jbig2enc.available(), + 'pngquant': pngquant.available(), + } + for name, available in image_optimizers.items(): + if not available: + messages.append( + f"The optional dependency '{name}' was not found, so some image " + f"optimizations could not be attempted." + ) + return result_path, messages @hookimpl diff --git a/src/ocrmypdf/optimize.py b/src/ocrmypdf/optimize.py index 0d1f3761..a73d22bc 100644 --- a/src/ocrmypdf/optimize.py +++ b/src/ocrmypdf/optimize.py @@ -629,7 +629,7 @@ def optimize( options = context.options if options.optimize == 0: safe_symlink(input_file, output_file) - return + return output_file if options.jpeg_quality == 0: options.jpeg_quality = DEFAULT_JPEG_QUALITY if options.optimize < 3 else 40 diff --git a/src/ocrmypdf/pluginspec.py b/src/ocrmypdf/pluginspec.py index 528d033a..2f744f99 100644 --- a/src/ocrmypdf/pluginspec.py +++ b/src/ocrmypdf/pluginspec.py @@ -10,7 +10,15 @@ from abc import ABC, abstractmethod from argparse import ArgumentParser, Namespace from logging import Handler from pathlib import Path -from typing import TYPE_CHECKING, AbstractSet, List, NamedTuple, Optional +from typing import ( + TYPE_CHECKING, + AbstractSet, + List, + NamedTuple, + Optional, + Sequence, + Tuple, +) import pluggy @@ -467,7 +475,7 @@ def optimize_pdf( context: PdfContext, executor: Executor, linearize: bool, -) -> Path: +) -> Tuple[Path, Sequence[str]]: """Optimize a PDF after image, OCR and metadata processing. If the input_pdf is a PDF/A, the plugin should modify input_pdf in a way @@ -490,6 +498,10 @@ def optimize_pdf( Path: If optimization is successful, the hook should return ``output_file``. If optimization does not produce a smaller file, the hook should return ``input_file``. + Sequence[str]: Any comments that the plugin wishes to report to the user, + especially reasons it was not able to further optimize the file. For + example, the plugin could report that a required third party was not + installed, so a specific optimization was not attempted. Note: This is a :ref:`firstresult hook`. diff --git a/tests/test_validation.py b/tests/test_validation.py index 3c704f00..3bab73e7 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -145,22 +145,16 @@ def test_report_file_size(tmp_path, caplog): pdf.Root.Dummy2 = waste_of_space + waste_of_space pdf.save(out) - with patch('ocrmypdf._validation.jbig2enc.available', return_value=True), patch( - 'ocrmypdf._validation.pngquant.available', return_value=True - ): - vd.report_output_file_size(opts, in_, out) - assert 'No reason' in caplog.text + vd.report_output_file_size(opts, in_, out, ['The optional dependency...']) + assert 'optional dependency' in caplog.text caplog.clear() - with patch('ocrmypdf._validation.jbig2enc.available', return_value=False), patch( - 'ocrmypdf._validation.pngquant.available', return_value=True - ): - vd.report_output_file_size(opts, in_, out) - assert 'optional dependency' in caplog.text + vd.report_output_file_size(opts, in_, out, []) + assert 'No reason' in caplog.text caplog.clear() opts = make_opts(in_, out, optimize=0, output_type='pdf') - vd.report_output_file_size(opts, in_, out) + vd.report_output_file_size(opts, in_, out, ["Optimization was disabled."]) assert 'disabled' in caplog.text caplog.clear()