Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ImageFont error messages #8338

Merged
merged 18 commits into from
Sep 18, 2024

Conversation

yngvem
Copy link
Contributor

@yngvem yngvem commented Aug 30, 2024

I spent a longer than I'd like to admit trying to figure out why my fonts wouldn't load before I realised that I was using the wrong function. This PR should hopefully help someone else realise their mistake faster.

Changes proposed in this pull request:

  • Add more information in the font loading exception messages
  • Add links between ImageFont.load and ImageFont.truetype in the documentation

Documentation update:
image
and similar for truetype:
image

@yngvem yngvem changed the title Improve error messages Minor fix: Improve error messages Aug 30, 2024
@radarhere radarhere changed the title Minor fix: Improve error messages Minor fix: Improve ImageFont error messages Aug 31, 2024
@@ -224,7 +228,7 @@ def __init__(
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}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this also a source of confusion for you? I'm not sure why this wouldn't be suitably obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this happened more on auto-pilot while coding in the evening -- I pretty much always try to include the incorrect value in error messages if it fails on input-validation as I find it makes debugging quite a lot easier for me (especially if the error is several frames down the stack from where I'm working).

Feel free to remove it, no hard feelings there :)

@radarhere
Copy link
Member

I've created yngvem#1 with some suggestions.

@yngvem
Copy link
Contributor Author

yngvem commented Sep 1, 2024

I just merged the changes now

@yngvem
Copy link
Contributor Author

yngvem commented Sep 2, 2024

I'm unsure why the pipeline crashed, but it seems to be unrelated to the changes in this PR? Maybe just unlucky with the load on the node that was assigned to this pipeline run?

Tests/test_file_tiff.py::TestFileTiff::test_timeout FAILED               [ 48%]

        if not self.map:
            # sort tiles in file order
>           self.tile.sort(key=_tilesort)
E           Failed: Timeout >6.0s

/vpy3/lib/python3.12/site-packages/PIL/ImageFile.py:245: Failed

https://github.com/python-pillow/Pillow/actions/runs/10656884196/job/29544000694?pr=8338

@radarhere radarhere changed the title Minor fix: Improve ImageFont error messages Improve ImageFont error messages Sep 2, 2024
@radarhere
Copy link
Member

I've created yngvem#2 with some suggestions.

Sort extensions alphabetically in error message
@yngvem
Copy link
Contributor Author

yngvem commented Sep 10, 2024

Good ideas for changes!

src/PIL/ImageFont.py Outdated Show resolved Hide resolved
src/PIL/ImageFont.py Outdated Show resolved Hide resolved
Co-authored-by: Hugo van Kemenade <[email protected]>
src/PIL/ImageFont.py Outdated Show resolved Hide resolved
Co-authored-by: Andrew Murray <[email protected]>
src/PIL/ImageFont.py Outdated Show resolved Hide resolved
@hugovk hugovk merged commit 743ddc7 into python-pillow:main Sep 18, 2024
50 checks passed
@hugovk
Copy link
Member

hugovk commented Sep 18, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants