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 documentation of ImageDraw return values #6556

Merged
merged 6 commits into from
Oct 3, 2022

Conversation

radarhere
Copy link
Member

@radarhere radarhere commented Sep 1, 2022

Resolves #6555

@nulano
Copy link
Contributor

nulano commented Sep 1, 2022

Would it make sense for these to match the ImageFont descriptions? At least the textlength functions seems explained more clearly there.

@hugovk
Copy link
Member

hugovk commented Sep 3, 2022

Also suggested in #6555, I think it would really help people upgrade by giving clear code examples of how to replace deprecations.

Something along the lines of:

# Deprecated:
width, height = textsize("Hello World")

# Use instead:
width = textlength("Hello World")

# Or:
left, top, right, bottom = textbbox((0,0), "Hello World")
width = right - left
height = bottom - top

And on the deprecations page:

And possibly in the release notes and deprecated functions' docstrings, or for brevity/avoid repetition, they could refer to the deprecations pages to see examples:

@radarhere
Copy link
Member Author

Ok, I've pushed further commits to try and address both of those feedback items.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Looks good, thanks, just a suggestion to use Sphinx for cross-referencing.

https://docs.readthedocs.io/en/stable/guides/cross-referencing-with-sphinx.html

docs/reference/ImageDraw.rst Outdated Show resolved Hide resolved
docs/reference/ImageDraw.rst Outdated Show resolved Hide resolved
docs/reference/ImageDraw.rst Outdated Show resolved Hide resolved
src/PIL/ImageFont.py Outdated Show resolved Hide resolved
src/PIL/ImageFont.py Outdated Show resolved Hide resolved
src/PIL/ImageFont.py Outdated Show resolved Hide resolved
src/PIL/ImageFont.py Outdated Show resolved Hide resolved
src/PIL/ImageFont.py Outdated Show resolved Hide resolved
src/PIL/ImageFont.py Outdated Show resolved Hide resolved
radarhere and others added 2 commits September 6, 2022 16:18
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thanks!

@nulano Any more feedback?

@nulano
Copy link
Contributor

nulano commented Sep 7, 2022

I think it would be best to also mention anchors, but it would be worth first adding support for those in ImageFont also. Shouldn't take very long, I just wasn't sure how useful it would be before.

I'll put together a PR in a few days and include it with my suggestions for how to mention anchors.
@radarhere Would you mind marking this a draft for now?

@radarhere radarhere marked this pull request as draft September 7, 2022 22:23
@radarhere
Copy link
Member Author

@nulano any updates?

@radarhere radarhere marked this pull request as ready for review October 2, 2022 04:40
@radarhere radarhere merged commit fea604f into python-pillow:main Oct 3, 2022
@radarhere radarhere deleted the returns branch October 3, 2022 08:07
@nulano
Copy link
Contributor

nulano commented Oct 25, 2022

@nulano any updates?

I did get the Python changes done, but did not have time to finish the tests before release.

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.

Make documentation precise
3 participants