bpo-30190: improved error msg for assertAlmostEqual(delta=...)#1331
Conversation
…e difference between the 2 numbers in case of failure
|
@giampaolo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @benjaminp, @voidspace and @serhiy-storchaka to be potential reviewers. |
| safe_repr(first), | ||
| safe_repr(second), | ||
| safe_repr(delta), | ||
| diff) |
There was a problem hiding this comment.
good catch, thanks
|
|
||
| if delta is not None: | ||
| if abs(first - second) <= delta: | ||
| diff = abs(first - second) |
There was a problem hiding this comment.
This line is duplicated in both branches. It could be move above the if.
| standardMsg = '%s != %s within %r places' % (safe_repr(first), | ||
| safe_repr(second), | ||
| places) | ||
| standardMsg = '%s != %s within %r places (%s difference)' % ( |
There was a problem hiding this comment.
What about assertNotAlmostEqual()?
There was a problem hiding this comment.
Maybe I'm understanding the code wrong but assertNotAlmostEqual fails if the two numbers are almost equal and just "off" by a little. So in case of:
assertNotAlmostEqual(1.00000001, 1.0, places=7)
The failure will look like this:
AssertionError 1.00000001 == 1.0 within 7 places
...which is already enough. Makes sense?
There was a problem hiding this comment.
How this differs from the case when delta is specified?
Actually I don't know what is the use case for assertNotAlmostEqual(). It is not used in Python tests (except testing assertNotAlmostEqual() itself). I suggest to not change it at all.
There was a problem hiding this comment.
To my understanding delta is entirely about acceptable difference (hence it makes sense to show the absolute diff) whereas places is about decimal precision.
I'm also not sure what's the use case for assertNotAlmostEqual() but whoever uses it will find this:
self.assertNotAlmostEqual(5, 10, delta=15)
...produces:
AssertionError: 5 == 10 within 15 delta (5 difference)
...more readable than the current message
So in summary, for assertNotAlmostEqual, I'd be for showing the difference if delta is specified but not if places is specified.
|
OK to merge? |
|
Merging as per Raymond's approval. |
5 participants
http://bugs.python.org/issue30190