Fix --jpeg-quality/--jpg-quality being dropped by the CLI (closes #1723)

namespace_to_options() only copied argparse namespace keys that were
literal members of OcrOptions.model_fields. The CLI dest for both
--jpeg-quality and --jpg-quality was jpeg_quality, but the pydantic field
was named jpg_quality (jpeg_quality existed only as a compatibility
property, absent from model_fields). The value was silently dropped into
extra_attrs, and the optimizer always fell back to its own hardcoded
default regardless of the flag.

The same alias mismatch also affected the Python API: create_options()
uses the same model_fields-matching logic as namespace_to_options(), so
ocrmypdf.ocr(jpeg_quality=...) was silently dropped too - only the
canonical jpg_quality= kwarg worked.

Rather than patch around the mismatch, consolidate on a single canonical
name: OcrOptions.jpeg_quality (matching the primary --jpeg-quality CLI
flag and the already-consistent naming in OptimizeOptions). jpg_quality
becomes a deprecated compatibility property, and ocrmypdf.ocr(jpg_quality=)
is a deprecated alias that warns and forwards to jpeg_quality via a new
create_options() remap step. --jpg-quality remains a working (already
hidden) CLI alias with no code-path divergence, since it now shares an
argparse dest that matches the field name directly.
This commit is contained in:
James R. Barlow
2026-07-27 23:14:27 -07:00
parent 5d49f75c56
commit 6f4744dd20
6 changed files with 112 additions and 27 deletions
+9 -15
View File
@@ -207,19 +207,19 @@ class OcrOptions(BaseModel):
# Optimization # Optimization
optimize: int = 1 optimize: int = 1
jpg_quality: int | None = None jpeg_quality: int | None = None
png_quality: int | None = None png_quality: int | None = None
# Compatibility alias for plugins that expect jpeg_quality # Deprecated compatibility alias for code that still uses the old field name
@property @property
def jpeg_quality(self): def jpg_quality(self):
"""Compatibility alias for jpg_quality.""" """Deprecated compatibility alias for jpeg_quality."""
return self.jpg_quality return self.jpeg_quality
@jpeg_quality.setter @jpg_quality.setter
def jpeg_quality(self, value): def jpg_quality(self, value):
"""Compatibility alias for jpg_quality.""" """Deprecated compatibility alias for jpeg_quality."""
self.jpg_quality = value self.jpeg_quality = value
# Output behavior # Output behavior
no_overwrite: bool = False no_overwrite: bool = False
@@ -642,12 +642,6 @@ class OcrOptions(BaseModel):
value = self.optimize value = self.optimize
if value is not None: if value is not None:
kwargs[field_name] = _convert_value(value) kwargs[field_name] = _convert_value(value)
elif namespace == 'optimize' and field_name == 'jpeg_quality':
# jpg_quality maps to jpeg_quality
if 'jpg_quality' in OcrOptions.model_fields:
value = self.jpg_quality
if value is not None:
kwargs[field_name] = _convert_value(value)
# Create and cache the plugin options instance # Create and cache the plugin options instance
instance = model_class(**kwargs) instance = model_class(**kwargs)
+25 -3
View File
@@ -345,6 +345,23 @@ def _remap_language_to_languages(options_kwargs: dict) -> None:
del options_kwargs['language'] del options_kwargs['language']
def _remap_jpg_quality_to_jpeg_quality(options_kwargs: dict) -> None:
"""Map the deprecated 'jpg_quality' parameter to 'jpeg_quality'.
'jpg_quality' was the original API parameter name. 'jpeg_quality' is the
canonical OcrOptions field, matching the primary --jpeg-quality CLI flag.
Prefer an explicitly-given 'jpeg_quality' if both are set.
"""
if 'jpg_quality' not in options_kwargs:
return
old_value = options_kwargs.pop('jpg_quality')
if old_value is None:
return
warn("ocrmypdf.ocr(jpg_quality=...) is deprecated, use jpeg_quality= instead.")
if options_kwargs.get('jpeg_quality') is None:
options_kwargs['jpeg_quality'] = old_value
def create_options( def create_options(
*, input_file: PathOrIO, output_file: PathOrIO, parser: ArgumentParser, **kwargs *, input_file: PathOrIO, output_file: PathOrIO, parser: ArgumentParser, **kwargs
) -> OcrOptions: ) -> OcrOptions:
@@ -369,6 +386,9 @@ def create_options(
# Map API parameter 'language' to OcrOptions field 'languages' # Map API parameter 'language' to OcrOptions field 'languages'
_remap_language_to_languages(options_kwargs) _remap_language_to_languages(options_kwargs)
# Map deprecated 'jpg_quality' parameter to 'jpeg_quality'
_remap_jpg_quality_to_jpeg_quality(options_kwargs)
# Set input and output files # Set input and output files
options_kwargs['input_file'] = input_file options_kwargs['input_file'] = input_file
options_kwargs['output_file'] = output_file options_kwargs['output_file'] = output_file
@@ -448,7 +468,8 @@ def ocr(
redo_ocr: bool | None = None, redo_ocr: bool | None = None,
skip_big: float | None = None, skip_big: float | None = None,
optimize: int | None = None, optimize: int | None = None,
jpg_quality: int | None = None, jpeg_quality: int | None = None,
jpg_quality: int | None = None, # Deprecated, use jpeg_quality instead
png_quality: int | None = None, png_quality: int | None = None,
jbig2_lossy: bool | None = None, jbig2_lossy: bool | None = None,
jbig2_page_group_size: int | None = None, jbig2_page_group_size: int | None = None,
@@ -511,7 +532,8 @@ def ocr( # noqa: D417
redo_ocr: bool | None = None, # Legacy, use mode='redo' instead redo_ocr: bool | None = None, # Legacy, use mode='redo' instead
skip_big: float | None = None, skip_big: float | None = None,
optimize: int | None = None, optimize: int | None = None,
jpg_quality: int | None = None, jpeg_quality: int | None = None,
jpg_quality: int | None = None, # Deprecated, use jpeg_quality instead
png_quality: int | None = None, png_quality: int | None = None,
jbig2_lossy: bool | None = None, # Deprecated, ignored jbig2_lossy: bool | None = None, # Deprecated, ignored
jbig2_page_group_size: int | None = None, # Deprecated, ignored jbig2_page_group_size: int | None = None, # Deprecated, ignored
@@ -883,7 +905,7 @@ def _hocr_to_ocr_pdf( # noqa: D417
jobs: int | None = None, jobs: int | None = None,
use_threads: bool | None = None, use_threads: bool | None = None,
optimize: int | None = None, optimize: int | None = None,
jpg_quality: int | None = None, jpeg_quality: int | None = None,
png_quality: int | None = None, png_quality: int | None = None,
jbig2_lossy: bool | None = None, # Deprecated, ignored jbig2_lossy: bool | None = None, # Deprecated, ignored
jbig2_page_group_size: int | None = None, # Deprecated, ignored jbig2_page_group_size: int | None = None, # Deprecated, ignored
+7 -7
View File
@@ -449,12 +449,12 @@ def convert_to_jbig2(
def _optimize_jpeg( def _optimize_jpeg(
xref: Xref, in_jpg: Path, opt_jpg: Path, jpg_quality: int xref: Xref, in_jpg: Path, opt_jpg: Path, jpeg_quality: int
) -> tuple[Xref, Path | None]: ) -> tuple[Xref, Path | None]:
with Image.open(in_jpg) as im: with Image.open(in_jpg) as im:
save_kwargs: dict[str, Any] = {'optimize': True} save_kwargs: dict[str, Any] = {'optimize': True}
if isinstance(jpg_quality, int) and 0 < jpg_quality <= 100: if isinstance(jpeg_quality, int) and 0 < jpeg_quality <= 100:
save_kwargs['quality'] = jpg_quality save_kwargs['quality'] = jpeg_quality
im.save(opt_jpg, **save_kwargs) im.save(opt_jpg, **save_kwargs)
if opt_jpg.stat().st_size > in_jpg.stat().st_size: if opt_jpg.stat().st_size > in_jpg.stat().st_size:
@@ -473,7 +473,7 @@ def transcode_jpegs(
for xref in jpegs: for xref in jpegs:
in_jpg = jpg_name(root, xref) in_jpg = jpg_name(root, xref)
opt_jpg = in_jpg.with_suffix('.opt.jpg') opt_jpg = in_jpg.with_suffix('.opt.jpg')
yield xref, in_jpg, opt_jpg, options.jpg_quality yield xref, in_jpg, opt_jpg, options.jpeg_quality
def finish_jpeg(result: tuple[Xref, Path | None], pbar: ProgressBar): def finish_jpeg(result: tuple[Xref, Path | None], pbar: ProgressBar):
xref, opt_jpg = result xref, opt_jpg = result
@@ -703,8 +703,8 @@ def optimize(
safe_symlink(input_file, output_file) safe_symlink(input_file, output_file)
return output_file return output_file
if not options.jpg_quality: if not options.jpeg_quality:
options.jpg_quality = DEFAULT_JPEG_QUALITY if options.optimize < 3 else 40 options.jpeg_quality = DEFAULT_JPEG_QUALITY if options.optimize < 3 else 40
if not options.png_quality: if not options.png_quality:
options.png_quality = DEFAULT_PNG_QUALITY if options.optimize < 3 else 30 options.png_quality = DEFAULT_PNG_QUALITY if options.optimize < 3 else 30
@@ -766,7 +766,7 @@ def main(infile, outfile, level, jobs=1):
output_file=outfile, # Required field output_file=outfile, # Required field
jobs=jobs, jobs=jobs,
optimize=int(level), optimize=int(level),
jpg_quality=0, # Use default jpeg_quality=0, # Use default
png_quality=0, png_quality=0,
jbig2_threshold=0.85, jbig2_threshold=0.85,
quiet=True, quiet=True,
+39
View File
@@ -79,6 +79,45 @@ def test_language_parameter_mapped_to_languages():
assert options.languages == ['eng', 'spa'] assert options.languages == ['eng', 'spa']
def test_jpeg_quality_parameter_reaches_options():
"""The canonical 'jpeg_quality' API parameter must reach OcrOptions.
Regression test for GitHub issue #1723: --jpeg-quality was silently
dropped by the CLI's namespace_to_options() because the OcrOptions field
was named jpg_quality. create_options(), used by the Python API, has the
same field-name matching logic and is affected the same way when passed
the alias name.
"""
from ocrmypdf.api import create_options, setup_plugin_infrastructure
from ocrmypdf.cli import get_parser
setup_plugin_infrastructure()
parser = get_parser()
options = create_options(
input_file='test.pdf', output_file='output.pdf', parser=parser, jpeg_quality=10
)
assert options.jpeg_quality == 10
def test_jpg_quality_parameter_deprecated_alias():
"""The old 'jpg_quality' API parameter still works but warns."""
from ocrmypdf.api import create_options, setup_plugin_infrastructure
from ocrmypdf.cli import get_parser
setup_plugin_infrastructure()
parser = get_parser()
with pytest.warns(UserWarning, match='jpg_quality'):
options = create_options(
input_file='test.pdf',
output_file='output.pdf',
parser=parser,
jpg_quality=42,
)
assert options.jpeg_quality == 42
def test_stream_api(resources: Path): def test_stream_api(resources: Path):
in_ = (resources / 'graph.pdf').open('rb') in_ = (resources / 'graph.pdf').open('rb')
out = BytesIO() out = BytesIO()
+23
View File
@@ -17,6 +17,7 @@ from PIL import Image, ImageDraw
from ocrmypdf import optimize as opt from ocrmypdf import optimize as opt
from ocrmypdf._exec import jbig2enc, pngquant from ocrmypdf._exec import jbig2enc, pngquant
from ocrmypdf._exec.ghostscript import rasterize_pdf from ocrmypdf._exec.ghostscript import rasterize_pdf
from ocrmypdf.cli import get_options_and_plugins
from ocrmypdf.helpers import IMG2PDF_KWARGS, Resolution from ocrmypdf.helpers import IMG2PDF_KWARGS, Resolution
from ocrmypdf.optimize import PdfImage, extract_image_filter from ocrmypdf.optimize import PdfImage, extract_image_filter
from ocrmypdf.pluginspec import GhostscriptRasterDevice from ocrmypdf.pluginspec import GhostscriptRasterDevice
@@ -81,6 +82,28 @@ def test_jpg_png_params(resources, outpdf):
) )
def test_jpeg_quality_cli_flag_reaches_options(resources, outpdf):
# Regression test for #1723: --jpeg-quality was silently dropped by
# namespace_to_options() because the argparse dest ('jpeg_quality') did
# not match the OcrOptions field it was checked against.
input_ = fspath(resources / 'c02-22.pdf')
options, _pm = get_options_and_plugins(
['--jpeg-quality', '10', input_, fspath(outpdf)]
)
assert options.jpeg_quality == 10
def test_jpg_quality_cli_alias_reaches_options(resources, outpdf):
# --jpg-quality is a hidden alias for --jpeg-quality (same argparse dest).
input_ = fspath(resources / 'c02-22.pdf')
options, _pm = get_options_and_plugins(
['--jpg-quality', '42', input_, fspath(outpdf)]
)
assert options.jpeg_quality == 42
# The old field name is still readable as a deprecated compatibility alias.
assert options.jpg_quality == 42
@needs_jbig2enc @needs_jbig2enc
def test_jbig2_lossless(resources, outpdf): def test_jbig2_lossless(resources, outpdf):
"""Test that JBIG2 lossless encoding works without JBIG2Globals.""" """Test that JBIG2 lossless encoding works without JBIG2Globals."""
+9 -2
View File
@@ -89,8 +89,15 @@ def test_mutex_options():
make_ocr_opts(redo_ocr=True, force_ocr=True) make_ocr_opts(redo_ocr=True, force_ocr=True)
def test_optimizing(caplog): def test_optimizing_png_quality_warns(caplog):
vd.check_options(*make_opts_pm(optimize=0, png_quality=18, jpeg_quality=10)) vd.check_options(*make_opts_pm(optimize=0, png_quality=18))
assert 'will be ignored because' in caplog.text
def test_optimizing_jpeg_quality_warns(caplog):
# Isolated from png_quality so this actually exercises the jpeg_quality
# path rather than being confounded by png_quality also being set.
vd.check_options(*make_opts_pm(optimize=0, jpeg_quality=10))
assert 'will be ignored because' in caplog.text assert 'will be ignored because' in caplog.text