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

unittest: Improve self.assert(Not)AlmostEqual(s) #8066

Merged
merged 11 commits into from
Jun 14, 2022

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Jun 12, 2022

first and second can be any values where second can be subtracted from first, and the result of this operation can be safely passed to abs(), and the result of the absing can be safely compared with float. https://github.com/python/cpython/blob/93310879665368445f95df2f3d9be7fb82435248/Lib/unittest/case.py#L904

There was also an additional error in the stub: places and delta cannot both be non-None. If they are, the runtime raises an exception.

Fixes #7891

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@AlexWaygood AlexWaygood marked this pull request as ready for review June 12, 2022 21:56
@github-actions

This comment has been minimized.

@AlexWaygood AlexWaygood changed the title unittest: Improve self.assertAlmostEqual unittest: Improve self.assert(Not)AlmostEqual(s) Jun 12, 2022
@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@Akuli
Copy link
Collaborator

Akuli commented Jun 13, 2022

How exactly are datetimes supposed to work?

>>> import unittest
>>> import datetime
>>> c=unittest.TestCase()
>>> c.assertAlmostEqual(datetime.datetime.now(), datetime.datetime.now())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.9/unittest/case.py", line 876, in assertAlmostEqual
    if round(diff, places) == 0:
TypeError: type datetime.timedelta doesn't define __round__ method

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Jun 13, 2022

How exactly are datetimes supposed to work?

>>> import unittest
>>> import datetime
>>> c=unittest.TestCase()
>>> c.assertAlmostEqual(datetime.datetime.now(), datetime.datetime.now())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.9/unittest/case.py", line 876, in assertAlmostEqual
    if round(diff, places) == 0:
TypeError: type datetime.timedelta doesn't define __round__ method

Good spot. Looks like it only works if delta is specified:

>>> import unittest
>>> import datetime
>>> c = unittest.TestCase()
>>> d1 = datetime.datetime.now()
>>> d2 = datetime.datetime.now()
>>> c.assertAlmostEqual(d1, d2, delta=datetime.timedelta(hours=1))
>>> 

@AlexWaygood AlexWaygood marked this pull request as draft June 13, 2022 15:51
@github-actions

This comment has been minimized.

@AlexWaygood AlexWaygood marked this pull request as ready for review June 13, 2022 22:07
@AlexWaygood AlexWaygood requested a review from Akuli June 13, 2022 22:07
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉


case.assertNotAlmostEqual(2.4, 2.41, places=9, delta=0.02) # type: ignore[call-overload]
case.assertNotAlmostEqual("foo", "bar") # type: ignore[call-overload]
case.assertNotAlmostEqual(datetime(1999, 1, 2), datetime(1999, 1, 2, microsecond=1)) # type: ignore[arg-type]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple thoughts about our tests in general:

  • The tests work very well for pull requests like this one :) I was going to write my own tests as part of reviewing this, but it was nice that they already existed.
  • I ran this test file with Python, making sure to get a runtime error whenever there's a type ignore comment. Ideally we would automate that somehow, but it doesn't seem to be worth the effort because we don't have that many tests.
  • The error codes in tests are pretty much noise to me. I ignore them. I don't really care whether "wrong argument types" is a call-overload or arg-type error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran this test file with Python, making sure to get a runtime error whenever there's a type ignore comment. Ideally we would automate that somehow, but it doesn't seem to be worth the effort because we don't have that many tests.

Oh, interesting idea. Tests like this file especially are basically very doctest-esque, just without the >>> prompt.

The error codes in tests are pretty much noise to me. I ignore them. I don't really care whether "wrong argument types" is a call-overload or arg-type error.

Yes, and compounding this problem, there's the fact that we generally only include tests for the trickiest functions to write stubs for. These are usually the ones where mypy ends up emitting really weird error codes. E.g. loads of the ones in test_sum.py are [list-item], and that's really meaningless; we shouldn't be asserting that mypy should emit such a meaningless error code (and there's no real reason why we should be giving mypy special treatment in these tests either). It seemed like a good idea at the time, but it's probably time to call it a day on the error-codes-in-tests.

@Akuli Akuli merged commit 5add91d into python:master Jun 14, 2022
@AlexWaygood AlexWaygood deleted the almostequal branch June 14, 2022 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assert(Not)AlmostEqual(s) specifies the type as float
2 participants