-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Conversation
dc6a3b3
to
cdadf93
Compare
@@ -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}" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
I've created yngvem#1 with some suggestions. |
Renamed variable for first part of splitext to root
I just merged the changes now |
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?
https://github.com/python-pillow/Pillow/actions/runs/10656884196/job/29544000694?pr=8338 |
I've created yngvem#2 with some suggestions. |
Sort extensions alphabetically in error message
Good ideas for changes! |
Co-authored-by: Andrew Murray <[email protected]>
for more information, see https://pre-commit.ci
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Andrew Murray <[email protected]>
Thank you! |
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:
ImageFont.load
andImageFont.truetype
in the documentationDocumentation update:
and similar for truetype: