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

Jump to bottom

turtle: tests for Vec2D.__abs__ are too strict #88897

Closed
mdickinson opened this issue Jul 24, 2021 · 12 comments
Closed

turtle: tests for Vec2D.__abs__ are too strict #88897

mdickinson opened this issue Jul 24, 2021 · 12 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes easy type-bug An unexpected behavior, bug, or error

Comments

@mdickinson
Copy link
Member

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
    Copy link
    Member Author

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

    @mdickinson mdickinson added 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error easy labels Jul 24, 2021
    @ambv
    Copy link
    Contributor

    ambv commented Jul 26, 2021

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

    @ambv
    Copy link
    Contributor

    ambv commented Jul 26, 2021

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

    @ambv
    Copy link
    Contributor

    ambv commented Jul 26, 2021

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

    @ambv ambv closed this as completed Jul 26, 2021
    @ambv ambv closed this as completed Jul 26, 2021
    @ambv
    Copy link
    Contributor

    ambv commented Jul 26, 2021

    Thanks! ✨ 🍰 ✨

    @encukou
    Copy link
    Member

    encukou commented Jan 27, 2022

    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 encukou reopened this Jan 27, 2022
    @encukou encukou reopened this Jan 27, 2022
    @encukou
    Copy link
    Member

    encukou commented Jan 27, 2022

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

    @miss-islington
    Copy link
    Contributor

    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
    Copy link
    Contributor

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

    @encukou encukou closed this as completed Jan 27, 2022
    @encukou encukou closed this as completed Jan 27, 2022
    @mdickinson
    Copy link
    Member Author

    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
    Copy link
    Member Author

    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
    Copy link
    Member Author

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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes easy type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @ambv @encukou @mdickinson @miss-islington