turtle: tests for Vec2D.__abs__ are too strict

[mdickinson]

BPO	44734
Nosy	@mdickinson, @encukou, @ambv, @miss-islington, @loganasherjones, @befeleme
PRs	bpo-44734: Fix precision in turtle tests #27343 [3.10] bpo-44734: Fix precision in turtle tests (GH-27343) #27361 [3.9] bpo-44734: Fix precision in turtle tests (GH-27343) #27362 bpo-44734: Fix floating point precision in test_turtle #30910 [3.10] bpo-44734: Fix floating point precision in test_turtle (GH-30910) #30960 [3.9] bpo-44734: Fix floating point precision in test_turtle (GH-30910) #30961

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2022-01-27.14:28:59.251>
created_at = <Date 2021-07-24.16:46:47.450>
labels = ['easy', 'type-bug', '3.9', '3.10', '3.11']
title = 'turtle: tests for Vec2D.__abs__ are too strict'
updated_at = <Date 2022-01-27.18:11:41.628>
user = 'https://github.com/mdickinson'

bugs.python.org fields:

activity = <Date 2022-01-27.18:11:41.628>
actor = 'mark.dickinson'
assignee = 'none'
closed = True
closed_date = <Date 2022-01-27.14:28:59.251>
closer = 'petr.viktorin'
components = []
creation = <Date 2021-07-24.16:46:47.450>
creator = 'mark.dickinson'
dependencies = []
files = []
hgrepos = []
issue_num = 44734
keywords = ['patch', 'easy']
message_count = 12.0
messages = ['398166', '398233', '398235', '398236', '398237', '411870', '411876', '411880', '411881', '411909', '411911', '411912']
nosy_count = 6.0
nosy_names = ['mark.dickinson', 'petr.viktorin', 'lukasz.langa', 'miss-islington', 'loganasherjones', 'ksurma']
pr_nums = ['27343', '27361', '27362', '30910', '30960', '30961']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue44734'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

[mdickinson]

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:

cpython/Lib/test/test_turtle.py

Lines 237 to 248 in 8158e05

	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)

The first test was reported as failing in issue bpo-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).

[ambv]

New changeset 3f135c0 by Logan Jones in branch 'main':
bpo-44734: Fix precision in turtle tests (GH-27343)
3f135c0

[ambv]

New changeset 16a174f by Miss Islington (bot) in branch '3.10':
bpo-44734: Fix precision in turtle tests (GH-27343) (GH-27361)
16a174f

[ambv]

New changeset 7b2185b by Miss Islington (bot) in branch '3.9':
bpo-44734: Fix precision in turtle tests (GH-27343) (GH-27362)
7b2185b

[ambv]

Thanks! ✨ 🍰 ✨

[encukou]

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: #27343 (review) )

Karolina's new PR is waiting for CLA confirmation.

[encukou]

New changeset aa78287 by Karolina Surma in branch 'main':
bpo-44734: Fix floating point precision in test_turtle (GH-30910)
aa78287

[miss-islington]

New changeset 486a4b3 by Miss Islington (bot) in branch '3.10':
bpo-44734: Fix floating point precision in test_turtle (GH-30910)
486a4b3

[miss-islington]

New changeset 8e98ccc by Miss Islington (bot) in branch '3.9':
bpo-44734: Fix floating point precision in test_turtle (GH-30910)
8e98ccc

[mdickinson]

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])

[mdickinson]

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).

[mdickinson]

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 ...