-
-
Couldn't load subscription status.
- Fork 33.3k
gh-95371: Add support for other image formats(e.g. PNG) to the turtle… #95378
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
Conversation
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
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.
Some little spacing nits.
Also, any particular reason to list them in order PGM, PPM, GIF, and PNG?
I'd either put them in some sort of order of "popularity/ease of use" (e.g. "PNG, GIF, PGM, and PPM") or alphabetically ("GIF,PGM, PNG, and PPM").
Misc/NEWS.d/next/Library/2022-07-28-12-32-59.gh-issue-95371.F24IFC.rst
Outdated
Show resolved
Hide resolved
|
|
Merging
|
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.
Please could you add some versionchanged or versionadded to document the changes/additions at the end of each function?
The next version will be 3.12.
Docs: https://devguide.python.org/documentation/markup/#paragraph-level-markup
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@adorilson @hugovk I'll continue this work. Sorry for being too late. I updated the document by adding the versionchanged information. Can you review the change? |
Great news, @relent95 . Now, the recommendation is that you use |
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.
Thanks, nearly there :)
Misc/NEWS.d/next/Library/2022-07-28-12-32-59.gh-issue-95371.F24IFC.rst
Outdated
Show resolved
Hide resolved
|
@bedevere-bot I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @hugovk: please review the changes made to this pull request. |
Co-authored-by: Hugo van Kemenade <[email protected]>
|
@bedevere-bot I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @hugovk: please review the changes made to this pull request. |
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.
Thank you!
Shape.__init__()The existing buggy and dead code calling an instance method
TurtleScreen._image()as a class method was removed.(It has never been reached viaturtle.register_shape())TurtleScreen.register_shape()The existing redundant code checking the filename extension was removed because the backend tkinter.PhotoImage does that.
A new pattern for arguments, such as
register_shape("turtle", "turtle.gif")was added to be consistent with cases for polygon shapes.(It will be useful when the filename is quite long, as in case of using a full path.)TurtleScreen.bgpic()This already supports other image formats. So only the documentation was changed.