From cdadf931e36cb3816096197ce91abfbe6b11df92 Mon Sep 17 00:00:00 2001 From: Yngve Mardal Moe Date: Fri, 30 Aug 2024 20:36:22 +0000 Subject: [PATCH 01/13] Improve error messages --- Tests/test_imagefont.py | 27 +++++++++++++++++++++++++++ src/PIL/ImageFont.py | 19 +++++++++++++++---- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/Tests/test_imagefont.py b/Tests/test_imagefont.py index 340cc4742..e2a7b759a 100644 --- a/Tests/test_imagefont.py +++ b/Tests/test_imagefont.py @@ -460,6 +460,17 @@ def test_free_type_font_get_mask(font: ImageFont.FreeTypeFont) -> None: assert mask.size == (108, 13) +def test_load_raises_if_image_not_found(tmp_path) -> None: + font_path = tmp_path / "file.font" + font_path.write_bytes(b"") + with pytest.raises(OSError) as excinfo: + ImageFont.load(font_path) + + pre = tmp_path / "file" + msg = f"cannot find glyph data file {pre}.{{png|gif|pbm}}" + assert msg in str(excinfo.value) + + def test_load_path_not_found() -> None: # Arrange filename = "somefilenamethatdoesntexist.ttf" @@ -471,6 +482,22 @@ def test_load_path_not_found() -> None: ImageFont.truetype(filename) +def test_load_path_exisitng_path(tmp_path) -> None: + # First, the file doens't exist, so we don't suggest `load` + some_path = tmp_path / "file.ttf" + with pytest.raises(OSError) as excinfo: + ImageFont.load_path(str(some_path)) + assert str(some_path) in str(excinfo.value) + assert "did you mean" not in str(excinfo.value) + + # The file exists, so the error message suggests to use `load` instead + some_path.write_bytes(b"") + with pytest.raises(OSError) as excinfo: + ImageFont.load_path(str(some_path)) + assert str(some_path) in str(excinfo.value) + assert " did you mean" in str(excinfo.value) + + def test_load_non_font_bytes() -> None: with open("Tests/images/hopper.jpg", "rb") as f: with pytest.raises(OSError): diff --git a/src/PIL/ImageFont.py b/src/PIL/ImageFont.py index 2ab65bfef..072acc31f 100644 --- a/src/PIL/ImageFont.py +++ b/src/PIL/ImageFont.py @@ -98,11 +98,13 @@ class ImageFont: def _load_pilfont(self, filename: str) -> None: with open(filename, "rb") as fp: image: ImageFile.ImageFile | None = None + filename_body = os.path.splitext(filename)[0] + for ext in (".png", ".gif", ".pbm"): if image: image.close() try: - fullname = os.path.splitext(filename)[0] + ext + fullname = filename_body + ext image = Image.open(fullname) except Exception: pass @@ -112,7 +114,9 @@ class ImageFont: else: if image: image.close() - msg = "cannot find glyph data file" + + pre = filename_body + msg = f"cannot find glyph data file {pre}.{{png|gif|pbm}}" raise OSError(msg) self.file = fullname @@ -224,7 +228,7 @@ class FreeTypeFont: raise core.ex if size <= 0: - msg = "font size must be greater than 0" + msg = f"font size must be greater than 0, not {size}" raise ValueError(msg) self.path = font @@ -774,6 +778,8 @@ def load(filename: str) -> ImageFont: :param filename: Name of font file. :return: A font object. :exception OSError: If the file could not be read. + + .. seealso:: :py:func:`PIL.ImageFont.truetype` """ f = ImageFont() f._load_pilfont(filename) @@ -850,6 +856,8 @@ def truetype( :return: A font object. :exception OSError: If the file could not be read. :exception ValueError: If the font size is not greater than zero. + + .. seealso:: :py:func:`PIL.ImageFont.load` """ def freetype(font: StrOrBytesPath | BinaryIO | None) -> FreeTypeFont: @@ -927,7 +935,10 @@ def load_path(filename: str | bytes) -> ImageFont: return load(os.path.join(directory, filename)) except OSError: pass - msg = "cannot find font file" + msg = f"cannot find font file '{filename}' in `sys.path`" + if os.path.exists(filename): + msg += f" did you mean `ImageFont.load({filename})` instead?" + raise OSError(msg) From 95194a2050081169f0b7db02040da91b1994da5a Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 31 Aug 2024 20:51:26 +1000 Subject: [PATCH 02/13] Use tempfile.NamedTemporaryFile --- Tests/test_imagefont.py | 43 ++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/Tests/test_imagefont.py b/Tests/test_imagefont.py index e2a7b759a..80cd0d578 100644 --- a/Tests/test_imagefont.py +++ b/Tests/test_imagefont.py @@ -5,6 +5,7 @@ import os import re import shutil import sys +import tempfile from io import BytesIO from pathlib import Path from typing import Any, BinaryIO @@ -460,15 +461,15 @@ def test_free_type_font_get_mask(font: ImageFont.FreeTypeFont) -> None: assert mask.size == (108, 13) -def test_load_raises_if_image_not_found(tmp_path) -> None: - font_path = tmp_path / "file.font" - font_path.write_bytes(b"") - with pytest.raises(OSError) as excinfo: - ImageFont.load(font_path) +def test_load_when_image_not_found(tmp_path: Path) -> None: + tmpfile = tmp_path / "file.font" + tmpfile.write_bytes(b"") + tempfile = str(tmpfile) + with pytest.raises(OSError) as e: + ImageFont.load(tempfile) - pre = tmp_path / "file" - msg = f"cannot find glyph data file {pre}.{{png|gif|pbm}}" - assert msg in str(excinfo.value) + root = os.path.splitext(tempfile)[0] + assert str(e.value) == f"cannot find glyph data file {root}.{{png|gif|pbm}}" def test_load_path_not_found() -> None: @@ -476,26 +477,24 @@ def test_load_path_not_found() -> None: filename = "somefilenamethatdoesntexist.ttf" # Act/Assert - with pytest.raises(OSError): + with pytest.raises(OSError) as e: ImageFont.load_path(filename) + + # The file doesn't exist, so don't suggest `load` + assert filename in str(e.value) + assert "did you mean" not in str(e.value) with pytest.raises(OSError): ImageFont.truetype(filename) -def test_load_path_exisitng_path(tmp_path) -> None: - # First, the file doens't exist, so we don't suggest `load` - some_path = tmp_path / "file.ttf" - with pytest.raises(OSError) as excinfo: - ImageFont.load_path(str(some_path)) - assert str(some_path) in str(excinfo.value) - assert "did you mean" not in str(excinfo.value) +def test_load_path_existing_path() -> None: + with tempfile.NamedTemporaryFile() as tmp: + with pytest.raises(OSError) as e: + ImageFont.load_path(tmp.name) - # The file exists, so the error message suggests to use `load` instead - some_path.write_bytes(b"") - with pytest.raises(OSError) as excinfo: - ImageFont.load_path(str(some_path)) - assert str(some_path) in str(excinfo.value) - assert " did you mean" in str(excinfo.value) + # The file exists, so the error message suggests to use `load` instead + assert tmp.name in str(e.value) + assert " did you mean" in str(e.value) def test_load_non_font_bytes() -> None: From e0a75b6d695b7be82c090adb1896022e42d344ff Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 31 Aug 2024 19:43:35 +1000 Subject: [PATCH 03/13] Renamed variable for first part of splitext to root --- src/PIL/ImageFont.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/PIL/ImageFont.py b/src/PIL/ImageFont.py index 072acc31f..43e86a7ed 100644 --- a/src/PIL/ImageFont.py +++ b/src/PIL/ImageFont.py @@ -98,13 +98,13 @@ class ImageFont: def _load_pilfont(self, filename: str) -> None: with open(filename, "rb") as fp: image: ImageFile.ImageFile | None = None - filename_body = os.path.splitext(filename)[0] + root = os.path.splitext(filename)[0] for ext in (".png", ".gif", ".pbm"): if image: image.close() try: - fullname = filename_body + ext + fullname = root + ext image = Image.open(fullname) except Exception: pass @@ -115,8 +115,7 @@ class ImageFont: if image: image.close() - pre = filename_body - msg = f"cannot find glyph data file {pre}.{{png|gif|pbm}}" + msg = f"cannot find glyph data file {root}.{{png|gif|pbm}}" raise OSError(msg) self.file = fullname @@ -937,7 +936,7 @@ def load_path(filename: str | bytes) -> ImageFont: pass msg = f"cannot find font file '{filename}' in `sys.path`" if os.path.exists(filename): - msg += f" did you mean `ImageFont.load({filename})` instead?" + msg += f", did you mean `ImageFont.load({filename})` instead?" raise OSError(msg) From fcca8a3059f94640dee493164b54ec466c13ff74 Mon Sep 17 00:00:00 2001 From: Yngve Mardal Moe Date: Sat, 31 Aug 2024 19:03:11 +0000 Subject: [PATCH 04/13] Fix accidental indent --- Tests/test_imagefont.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/test_imagefont.py b/Tests/test_imagefont.py index e2a7b759a..80ec7bd6b 100644 --- a/Tests/test_imagefont.py +++ b/Tests/test_imagefont.py @@ -487,8 +487,8 @@ def test_load_path_exisitng_path(tmp_path) -> None: some_path = tmp_path / "file.ttf" with pytest.raises(OSError) as excinfo: ImageFont.load_path(str(some_path)) - assert str(some_path) in str(excinfo.value) - assert "did you mean" not in str(excinfo.value) + assert str(some_path) in str(excinfo.value) + assert "did you mean" not in str(excinfo.value) # The file exists, so the error message suggests to use `load` instead some_path.write_bytes(b"") From ef51e7a1c70ae684615724750c32ba3ca6923123 Mon Sep 17 00:00:00 2001 From: Yngve Mardal Moe Date: Sun, 1 Sep 2024 19:49:44 +0000 Subject: [PATCH 05/13] Fix wrong indentation for assert --- Tests/test_imagefont.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/test_imagefont.py b/Tests/test_imagefont.py index 80cd0d578..66e6947c2 100644 --- a/Tests/test_imagefont.py +++ b/Tests/test_imagefont.py @@ -480,9 +480,9 @@ def test_load_path_not_found() -> None: with pytest.raises(OSError) as e: ImageFont.load_path(filename) - # The file doesn't exist, so don't suggest `load` - assert filename in str(e.value) - assert "did you mean" not in str(e.value) + # The file doesn't exist, so don't suggest `load` + assert filename in str(e.value) + assert "did you mean" not in str(e.value) with pytest.raises(OSError): ImageFont.truetype(filename) From e14072e9738275ffe2898f32d36cbd2f3c687cf3 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 7 Sep 2024 19:08:07 +1000 Subject: [PATCH 06/13] Added further detail --- src/PIL/ImageFont.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/PIL/ImageFont.py b/src/PIL/ImageFont.py index 43e86a7ed..abfa4aa80 100644 --- a/src/PIL/ImageFont.py +++ b/src/PIL/ImageFont.py @@ -771,14 +771,13 @@ class TransposedFont: def load(filename: str) -> ImageFont: """ - Load a font file. This function loads a font object from the given - bitmap font file, and returns the corresponding font object. + Load a font file. This function loads a font object from the given + bitmap font file, and returns the corresponding font object. For loading TrueType + or OpenType fonts instead, see :py:func:`~PIL.ImageFont.truetype`. :param filename: Name of font file. :return: A font object. :exception OSError: If the file could not be read. - - .. seealso:: :py:func:`PIL.ImageFont.truetype` """ f = ImageFont() f._load_pilfont(filename) @@ -794,7 +793,8 @@ def truetype( ) -> FreeTypeFont: """ Load a TrueType or OpenType font from a file or file-like object, - and create a font object. + and create a font object. For loading bitmap fonts instead, + see :py:func:`~PIL.ImageFont.load` and :py:func:`~PIL.ImageFont.load_path`. This function loads a font object from the given file or file-like object, and creates a font object for a font of the given size. @@ -855,8 +855,6 @@ def truetype( :return: A font object. :exception OSError: If the file could not be read. :exception ValueError: If the font size is not greater than zero. - - .. seealso:: :py:func:`PIL.ImageFont.load` """ def freetype(font: StrOrBytesPath | BinaryIO | None) -> FreeTypeFont: From dbe979d032afed04b93f9ce1aaa9c7f8e23542b1 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 7 Sep 2024 19:09:01 +1000 Subject: [PATCH 07/13] Sort extensions alphabetically in error message --- Tests/test_imagefont.py | 2 +- src/PIL/ImageFont.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/test_imagefont.py b/Tests/test_imagefont.py index 66e6947c2..32cd1db8a 100644 --- a/Tests/test_imagefont.py +++ b/Tests/test_imagefont.py @@ -469,7 +469,7 @@ def test_load_when_image_not_found(tmp_path: Path) -> None: ImageFont.load(tempfile) root = os.path.splitext(tempfile)[0] - assert str(e.value) == f"cannot find glyph data file {root}.{{png|gif|pbm}}" + assert str(e.value) == f"cannot find glyph data file {root}.{{gif|pbm|png}}" def test_load_path_not_found() -> None: diff --git a/src/PIL/ImageFont.py b/src/PIL/ImageFont.py index abfa4aa80..b6d69019d 100644 --- a/src/PIL/ImageFont.py +++ b/src/PIL/ImageFont.py @@ -115,7 +115,7 @@ class ImageFont: if image: image.close() - msg = f"cannot find glyph data file {root}.{{png|gif|pbm}}" + msg = f"cannot find glyph data file {root}.{{gif|pbm|png}}" raise OSError(msg) self.file = fullname From 547832fd592ee1392533be5bf935c19c690a665f Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 7 Sep 2024 19:49:21 +1000 Subject: [PATCH 08/13] Use tempfile.NamedTemporaryFile --- Tests/test_imagefont.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/Tests/test_imagefont.py b/Tests/test_imagefont.py index 32cd1db8a..e3d847567 100644 --- a/Tests/test_imagefont.py +++ b/Tests/test_imagefont.py @@ -461,14 +461,15 @@ def test_free_type_font_get_mask(font: ImageFont.FreeTypeFont) -> None: assert mask.size == (108, 13) -def test_load_when_image_not_found(tmp_path: Path) -> None: - tmpfile = tmp_path / "file.font" - tmpfile.write_bytes(b"") - tempfile = str(tmpfile) +def test_load_when_image_not_found() -> None: + with tempfile.NamedTemporaryFile(delete=False) as tmp: + pass with pytest.raises(OSError) as e: - ImageFont.load(tempfile) + ImageFont.load(tmp.name) - root = os.path.splitext(tempfile)[0] + os.unlink(tmp.name) + + root = os.path.splitext(tmp.name)[0] assert str(e.value) == f"cannot find glyph data file {root}.{{gif|pbm|png}}" @@ -492,8 +493,8 @@ def test_load_path_existing_path() -> None: with pytest.raises(OSError) as e: ImageFont.load_path(tmp.name) - # The file exists, so the error message suggests to use `load` instead - assert tmp.name in str(e.value) + # The file exists, so the error message suggests to use `load` instead + assert tmp.name in str(e.value) assert " did you mean" in str(e.value) From d2efd7dd5fcd19b7cb7768fc90ffa51676ff5e7a Mon Sep 17 00:00:00 2001 From: Yngve Mardal Moe Date: Wed, 11 Sep 2024 10:32:42 +0200 Subject: [PATCH 09/13] Update src/PIL/ImageFont.py Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com> --- src/PIL/ImageFont.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PIL/ImageFont.py b/src/PIL/ImageFont.py index b6d69019d..7ee3aaa55 100644 --- a/src/PIL/ImageFont.py +++ b/src/PIL/ImageFont.py @@ -934,7 +934,7 @@ def load_path(filename: str | bytes) -> ImageFont: pass msg = f"cannot find font file '{filename}' in `sys.path`" if os.path.exists(filename): - msg += f", did you mean `ImageFont.load({filename})` instead?" + msg += f", did you mean `ImageFont.load(\"{filename}\")` instead?" raise OSError(msg) From e4f13020e16925f1dcfe738218e4d03a3c3992bb Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 11 Sep 2024 08:33:09 +0000 Subject: [PATCH 10/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/PIL/ImageFont.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PIL/ImageFont.py b/src/PIL/ImageFont.py index 7ee3aaa55..55ab61951 100644 --- a/src/PIL/ImageFont.py +++ b/src/PIL/ImageFont.py @@ -934,7 +934,7 @@ def load_path(filename: str | bytes) -> ImageFont: pass msg = f"cannot find font file '{filename}' in `sys.path`" if os.path.exists(filename): - msg += f", did you mean `ImageFont.load(\"{filename}\")` instead?" + msg += f', did you mean `ImageFont.load("{filename}")` instead?' raise OSError(msg) From 77503156b17a8314652dcd69941cbdb222dd7bcf Mon Sep 17 00:00:00 2001 From: Yngve Mardal Moe Date: Wed, 11 Sep 2024 14:22:45 +0200 Subject: [PATCH 11/13] Update src/PIL/ImageFont.py Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- src/PIL/ImageFont.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PIL/ImageFont.py b/src/PIL/ImageFont.py index 55ab61951..0fccee32f 100644 --- a/src/PIL/ImageFont.py +++ b/src/PIL/ImageFont.py @@ -932,9 +932,9 @@ def load_path(filename: str | bytes) -> ImageFont: return load(os.path.join(directory, filename)) except OSError: pass - msg = f"cannot find font file '{filename}' in `sys.path`" + msg = f"cannot find font file '{filename}' in sys.path" if os.path.exists(filename): - msg += f', did you mean `ImageFont.load("{filename}")` instead?' + msg += f', did you mean ImageFont.load("{filename}") instead?' raise OSError(msg) From 32c514d24ccac400b025c981bb27b48d6e78b2b6 Mon Sep 17 00:00:00 2001 From: Yngve Mardal Moe Date: Wed, 11 Sep 2024 14:54:53 +0200 Subject: [PATCH 12/13] Update src/PIL/ImageFont.py Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com> --- src/PIL/ImageFont.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PIL/ImageFont.py b/src/PIL/ImageFont.py index 0fccee32f..89efe5bd8 100644 --- a/src/PIL/ImageFont.py +++ b/src/PIL/ImageFont.py @@ -932,7 +932,7 @@ def load_path(filename: str | bytes) -> ImageFont: return load(os.path.join(directory, filename)) except OSError: pass - msg = f"cannot find font file '{filename}' in sys.path" + msg = f'cannot find font file "{filename}" in sys.path' if os.path.exists(filename): msg += f', did you mean ImageFont.load("{filename}") instead?' From 9adb476f37f28cd4a27bb66c6242e4d11925b53b Mon Sep 17 00:00:00 2001 From: Andrew Murray <3112309+radarhere@users.noreply.github.com> Date: Wed, 18 Sep 2024 23:58:23 +1000 Subject: [PATCH 13/13] Rearranged text --- src/PIL/ImageFont.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/PIL/ImageFont.py b/src/PIL/ImageFont.py index 70cd57de7..b694b817e 100644 --- a/src/PIL/ImageFont.py +++ b/src/PIL/ImageFont.py @@ -808,10 +808,10 @@ def truetype( ) -> FreeTypeFont: """ Load a TrueType or OpenType font from a file or file-like object, - and create a font object. For loading bitmap fonts instead, - see :py:func:`~PIL.ImageFont.load` and :py:func:`~PIL.ImageFont.load_path`. - This function loads a font object from the given file or file-like - object, and creates a font object for a font of the given size. + and create a font object. This function loads a font object from the given + file or file-like object, and creates a font object for a font of the given + size. For loading bitmap fonts instead, see :py:func:`~PIL.ImageFont.load` + and :py:func:`~PIL.ImageFont.load_path`. Pillow uses FreeType to open font files. On Windows, be aware that FreeType will keep the file open as long as the FreeTypeFont object exists. Windows