I think I addressed most of these issues in the summary note I jsut posted, but a few specific comments:


Unfortunately, I don't think we can change assertAlmostEqual. If we
change the implementation, tests which were passing may fail, and tests
which were failing may pass.

Agreed -- would have thought that was off the table. And ti's really a different test than the proposal -- it is an absolute_tolerance test, but where the tolerance is specified in number of decimal digits after the decimal point (or an optional specific delta) -- not really that useful , but I guess if you have assertTrue(), then why not?

But adding a relative tolerance to unittest makes a lot of sense -- would "assertCloseTo" sound entirely too much like assertAlmostEqual? I think it may be OK if the docs for each pointed to the other.
 
The actual fuzzy comparison itself is handled by a function
approx_equal(x, y, tol, rel).

NOTE: the difference between this and the my current PEP version is that that absolute tolerance defaults to something other than zero (though it looks like it does default to zero for the assert method), and it is a symmetric test (what Boost calls the "strong" test)

- somebody other than me should review NumericTestCase.assertApproxEqual
  and check that it does nothing unreasonable;

Well it requires the tolerance values to be set on the instance, and they default to zero. So if we were to add this to unittest.TestCase, would you make those instance attributes of TestCase? If so,  we want different defaults -- so people could call the assertion with defaults and get something useful. I suggest the one I put in my proposal, naturally ;-)

 I looked at the underlying function pretty closely. I don't see anything wrong with it. I did a few things differently:

- no need to check for NaN explicitly, the comparisons take care of that anyway.

- inspired by the boost approach - I used "and" and "or", rather than calling max() -- same result, slightly better performance.

But in the end -- essentially the same as the PEP code, except where it's intended to be different.
 
- since there are considerable disagreements about the right way to
  handle a fuzzy comparison when *both* an absolute and relative error
  are given, people who disagree with the default definition can simply
  subclass TestCase and redefine the approx_equal method.

(Which is much simpler than having to write the whole assertApproxEqual
method from scratch.)

what assertApproxEqual does is add the ability to test a while sequence of values -- much like numpy's all_close. Do any  of the other TestCase assertions provide that?

But you could also add an optional parameter to pass in an alternate comparison function, rather than have it be a method of TestCase. As I said, I think it's better to have it available, and discoverable, for use outside of unitest.
 
Note that in this case, at least, we do want a symmetric version of
"is_close", since neither guess nor new_guess is "correct", they are
both approximations.

true, but you are also asking the question -- is the new_guess much different that guess. Which points to a asymmetric test -- but either would work.

-Chris

--

Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R            (206) 526-6959   voice
7600 Sand Point Way NE   (206) 526-6329   fax
Seattle, WA  98115       (206) 526-6317   main reception

Chris.Barker@noaa.gov