This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: turtle: tests for Vec2D.__abs__ are too strict
Type: behavior Stage: resolved
Components: Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ksurma, loganasherjones, lukasz.langa, mark.dickinson, miss-islington, petr.viktorin
Priority: normal Keywords: easy, patch

Created on 2021-07-24 16:46 by mark.dickinson, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 27343 merged loganasherjones, 2021-07-25 00:22
PR 27361 merged miss-islington, 2021-07-26 15:21
PR 27362 merged miss-islington, 2021-07-26 15:21
PR 30910 merged ksurma, 2022-01-26 08:39
PR 30960 merged miss-islington, 2022-01-27 13:58
PR 30961 merged miss-islington, 2022-01-27 13:58
Messages (12)
msg398166 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2021-07-24 16:46
From the tests for Vec2D.__abs__ in the turtle module we have:

    def test_distance(self):
        vec = Vec2D(6, 8)
        expected = 10
        self.assertEqual(abs(vec), expected)


        vec = Vec2D(0, 0)
        expected = 0
        self.assertEqual(abs(vec), expected)


        vec = Vec2D(2.5, 6)
        expected = 6.5
        self.assertEqual(abs(vec), expected)

GitHub link: https://github.com/python/cpython/blob/8158e059e9952f08d19a18d3e9e021cee2393cd2/Lib/test/test_turtle.py#L237-L248

The first test was reported as failing in issue #44728, with error:

======================================================================
FAIL: test_distance (test.test_turtle.TestVec2D)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/build/python/src/Python-3.9.6/Lib/test/test_turtle.py", line 237, in test_distance
    self.assertEqual(abs(vec), expected)
AssertionError: 9.999999999999998 != 10

The first and last test should use assertAlmostEqual with a suitable tolerance (the default tolerance is probably fine).
msg398233 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-07-26 15:21
New changeset 3f135c073a53793ec68902f6b513934ddff47235 by Logan Jones in branch 'main':
bpo-44734: Fix precision in turtle tests (GH-27343)
https://github.com/python/cpython/commit/3f135c073a53793ec68902f6b513934ddff47235
msg398235 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-07-26 15:55
New changeset 16a174f7bac481ff6f859179b30a74d867747137 by Miss Islington (bot) in branch '3.10':
bpo-44734: Fix precision in turtle tests (GH-27343) (GH-27361)
https://github.com/python/cpython/commit/16a174f7bac481ff6f859179b30a74d867747137
msg398236 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-07-26 15:56
New changeset 7b2185b8e495daed30b50fe89f3ada0dc0129b62 by Miss Islington (bot) in branch '3.9':
bpo-44734: Fix precision in turtle tests (GH-27343) (GH-27362)
https://github.com/python/cpython/commit/7b2185b8e495daed30b50fe89f3ada0dc0129b62
msg398237 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-07-26 15:59
Thanks! ✨ 🍰 ✨
msg411870 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2022-01-27 13:30
> The first and last test should use assertAlmostEqual with a suitable tolerance (the default tolerance is probably fine).

The merged PR only added tolerance to the last test. On some architectures, the first test still fails.
(No one is to blame -- this comment should apply to one line but GitHub shows two lines as context: https://github.com/python/cpython/pull/27343#pullrequestreview-714345826 )

Karolina's new PR is waiting for CLA confirmation.
msg411876 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2022-01-27 13:58
New changeset aa78287bc6d1c4fc07ee134642eb72db67b771a0 by Karolina Surma in branch 'main':
bpo-44734: Fix floating point precision in test_turtle (GH-30910)
https://github.com/python/cpython/commit/aa78287bc6d1c4fc07ee134642eb72db67b771a0
msg411880 - (view) Author: miss-islington (miss-islington) Date: 2022-01-27 14:21
New changeset 486a4b382943ed4c965a0a36b177e8e0b083a6e5 by Miss Islington (bot) in branch '3.10':
bpo-44734: Fix floating point precision in test_turtle (GH-30910)
https://github.com/python/cpython/commit/486a4b382943ed4c965a0a36b177e8e0b083a6e5
msg411881 - (view) Author: miss-islington (miss-islington) Date: 2022-01-27 14:24
New changeset 8e98ccc4c3fd1a12f168466422d206d814eba0f9 by Miss Islington (bot) in branch '3.9':
bpo-44734: Fix floating point precision in test_turtle (GH-30910)
https://github.com/python/cpython/commit/8e98ccc4c3fd1a12f168466422d206d814eba0f9
msg411909 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2022-01-27 18:01
Low priority, but it may also be worth updating the implementation of `Vec2D.__abs__`. It currently looks like this:

    def __abs__(self):
        return (self[0]**2 + self[1]**2)**0.5

But would be more robust if it used hypot:

    def __abs__(self):
        return math.hypot(self[0], self[1])
msg411911 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2022-01-27 18:06
Apologies; looks like I'm out of date on this. It's already using hypot, which makes it more than a little worrying that it doesn't get the right answer for `Vec2D(6, 8)`.
msg411912 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2022-01-27 18:11
Sorry again, all; I failed to read everything that was going on here. The test *wasn't* failing with the hypot-based version of Vec2D.__abs__ that's in the main branch; only with the "**0.5"-based version that was still in the older branches. Please ignore this and the previous two messages ...
History
Date User Action Args
2022-04-11 14:59:47adminsetgithub: 88897
2022-01-27 18:11:41mark.dickinsonsetmessages: + msg411912
2022-01-27 18:06:27mark.dickinsonsetmessages: + msg411911
2022-01-27 18:01:36mark.dickinsonsetmessages: + msg411909
2022-01-27 14:28:59petr.viktorinsetstatus: open -> closed
stage: patch review -> resolved
2022-01-27 14:24:16miss-islingtonsetmessages: + msg411881
2022-01-27 14:21:21miss-islingtonsetmessages: + msg411880
2022-01-27 13:58:08miss-islingtonsetpull_requests: + pull_request29140
2022-01-27 13:58:03miss-islingtonsetstage: resolved -> patch review
pull_requests: + pull_request29139
2022-01-27 13:58:03petr.viktorinsetmessages: + msg411876
2022-01-27 13:30:21petr.viktorinsetstatus: closed -> open
nosy: + petr.viktorin
messages: + msg411870

2022-01-26 08:39:19ksurmasetnosy: + ksurma

pull_requests: + pull_request29089
2021-07-26 15:59:55lukasz.langasetmessages: + msg398237
2021-07-26 15:59:48lukasz.langasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-07-26 15:56:23lukasz.langasetmessages: + msg398236
2021-07-26 15:56:00lukasz.langasetmessages: + msg398235
2021-07-26 15:21:25miss-islingtonsetpull_requests: + pull_request25902
2021-07-26 15:21:19miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request25901
2021-07-26 15:21:17lukasz.langasetnosy: + lukasz.langa
messages: + msg398233
2021-07-25 00:22:31loganasherjonessetkeywords: + patch
nosy: + loganasherjones

pull_requests: + pull_request25885
stage: patch review
2021-07-24 16:49:25mark.dickinsonsetkeywords: + easy
2021-07-24 16:47:06mark.dickinsonsettype: behavior
versions: + Python 3.9, Python 3.10, Python 3.11
2021-07-24 16:46:47mark.dickinsoncreate