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

Improved type hints for ImageDraw and ImageFont #8715

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

AsfhtgkDavid
Copy link

@AsfhtgkDavid AsfhtgkDavid commented Jan 25, 2025

Fixes #typing

Changes proposed in this pull request:

  • Changed some str type to Literal for more specificity

@radarhere radarhere changed the title Improved typing for ImageDraw Improved type hints for ImageDraw and ImageFont Jan 27, 2025
Copy link
Member

@radarhere radarhere left a comment

Choose a reason for hiding this comment

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

Approved, but I'd like to know what @hugovk thinks of Anchor, Align and Direction as well.

I'm not sure if a list of 24 Anchor values is helpful or not. I would imagine either you already know what value you want, or you need to go and look at https://pillow.readthedocs.io/en/stable/handbook/text-anchors.html.

@AsfhtgkDavid
Copy link
Author

I think this type hinting is in line with The zen of python's principle ‘Explicit is better than implicit’.

@hugovk
Copy link
Member

hugovk commented Jan 31, 2025

How disruptive do you think this will be?

Does this mean people using the old string versions will start getting potentially lots of type hint errors, and will need to switch to the new literals or add type: ignore[arg-type]?

Is there any performance impact?

@radarhere
Copy link
Member

Does this mean people using the old string versions will start getting potentially lots of type hint errors, and will need to switch to the new literals or add type: ignore[arg-type]?

from PIL import ImageFont
font = ImageFont.load_default()
font.getbbox("test", anchor="la")

would continue to work without a problem, but

anchor = "la"
font.getbbox("test", anchor=anchor)

would raise a type hint error, yes.

Is there any performance impact?

I don't expect so. I would say that only type hinting code has been modified here.

How disruptive do you think this will be?

The type hints being changed were added in 11.0.0, so users who have updated in the last six months and use type hinting would be affected. If there is a feeling that this is troublesome to people who are excitedly getting on board with the idea of using type hints in Pillow, then I'm personally with closing this for that reason.

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

Successfully merging this pull request may close these issues.

3 participants