gh-126399 minor changes in RawTurtle.clone() method#126401
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Update RawTurtle.clone() method. 1) moved self._newLine(self._drawing) after deepcopy() 2) added q.items.append(q.currentLineItem) after q.currentLineItem = screen._createline() Reason for 1): When clear method is called on the cloned Turtle object, the method also deletes the first line of the original Turtle object which is drawn after cloning. Reason for 2): When clear method is called on the cloned Turtle object, the method does not delete the first line of the cloned Turtle object which is drawn after cloning
All commit authors signed the Contributor License Agreement. |
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Misc/NEWS.d/next/Library/2024-11-04-15-08-02.gh-issue-126399.-4rB22.rst Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
…4rB22.rst Co-authored-by: Nadeshiko Manju <[email protected]>
There was a problem hiding this comment.
- Fix the doctest failure (see comment). DONE
- Add a unittest that fails without the patch and passes with it.
EDIT: I thought of expanding the doc clone example to make it a true test, but it might require too much for readers.
I am not very familiar either with doctest or the turtle code. So not likely to make a final decision and merge.
Uh oh!
There was an error while loading. Please reload this page.
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 |
moved self._newLine(self._drawing) to original position changed q.items.append(q.currentLineItem) to q.items[-1] = q.currentLineItem
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
terryjreedy commented Nov 7, 2024
I added some info relevant to testing on the issue. I still need to play around with example on issue, but am done for today. |
added a unit test that fails without the patch and passes with it
revert change
TestRawTurtle.test_clone(): copied screen from TestTurtleScreen.test_save_raises_if_wrong_extension
TestRawTurtle.test_clone(): screen = Screen() as in Turtle class
Screen belongs to turtle namespace
check variable name
handling _tkinter.TclError
correct qualified name of TclError
raise SkipTest to skip test instead of skipTest()
correct qualified name of unittest.case.SkipTest exception class
lint: remove trailing whitespaces
Chaebin-Kim24 commented Nov 21, 2024
I added a unit test for the items of cloned turtle. This test passes with the patch. I still need to confirm that the test fails without the patch. |
The same test is confirmed to fail without the patch in #127189. |
Uh oh!
There was an error while loading. Please reload this page.
Misc/NEWS.d/next/Library/2024-11-04-15-08-02.gh-issue-126399.-4rB22.rst Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <[email protected]>
…4rB22.rst Co-authored-by: Hugo van Kemenade <[email protected]>
4 participants
This is a minor change of RawTurtle.clone() method to correct self.items of the cloned Turtle.
In clone() method, regarding items and currentLineItem,
self._newLine(self._drawing)q = self.deepcopy()q.currentLineItem = screen._createline()Potential problems of this code is that
To address these issues, following changes were made.