[docs] Doc: remove errors about mixed-type comparisons. (issue 12067)

andreas.r.maier at gmx.de andreas.r.maier at gmx.de
Mon Mar 2 18:56:14 CET 2015


Responded to Martin's comments.


https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py
File Lib/test/test_compare.py (right):

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode137
Lib/test/test_compare.py:137: class ComparisonTest2(unittest.TestCase):
The new test class I added is really supposed to test everything. The
existing test class also compares results to some extent, but does not
cover everything. I still did not want to remove it, just in case.
I'll change it as follows:
ComparisonTest1 -> ComparisonSimpleTest
ComparisonTest2 -> ComparisonFullTest
to express the expectation for the second test class.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode145
Lib/test/test_compare.py:145: class BaseCompare(object):
Will rename to CompBase, to be consistent with the other renamed classes
(see comment below).

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode162
Lib/test/test_compare.py:162: 
Will rename these classes e.g. from Class_eq to CompEq, so their names
are PEP-8 compliant.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode328
Lib/test/test_compare.py:328: def assert_insts(self, i1, i2, equal,
comp, i1_meth=(), i2_meth=()):
Will change name to assert_comparisons, for now. I will split up the two
_subtest methods into six, for better identification based on subTest()
(I really like that), but if I look at the number of calls to
assert_comparisons(), I don't think I'd like to duplicate them.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode340
Lib/test/test_compare.py:340: None means: Equality comparison not
supported
Agreed. I will remove None from the description and will add an
assertion that it is not passed as None.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode347
Lib/test/test_compare.py:347: None means: Order comparison not supported
I'll change the description of None to mean "Undefined result", plus
I'll add a clarifying note: "Note that the expected results shown above
just indicate which order comparison expression evaluates to True; they
do not make any statement about whether the ordering is a total
ordering."
Let me know if you think that does not cover it.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode375
Lib/test/test_compare.py:375: self.assertEqual(i1 == i2, id(i1) ==
id(i2))
I thought we would have cases where id(i1)-id(i2) needs to be passed for
order comparisons, so I wanted it to be consistent with that. However,
there are no such cases (the one that existed was in error and was
removed), so I think I agree and will change it as you proposed.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode480
Lib/test/test_compare.py:480: self.assert_insts(i1, i2, False,
id(i1)-id(i2))
Seems like that's a remainder from earlier versions of the code where I
still experimented. Because object types don't have a default order
comparison, the assert_insts() method ignores the value anyway and
asserts that TypeError is raised. I'll change the value that is passed,
to None in both of the cases shown here.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode511
Lib/test/test_compare.py:511: self.assert_insts(insts_a[0], insts_b[0],
True, 0,
The create_sorted_insts() method ensures that the returned objects are
in increasing order of their identities. The only interesting case is
where the value is descending between two objects whose identity is
ascending. That allows asserting that the order comparison is based on
values, not on identities.

Now, the flaw in the current code is that it compares always one object
of the first list with one object of the second list, so the identity
order is arbitrary between them. I will fix that by having an additional
loop through the classes with only one list. Also, the existing loop
across the classes can be simplified somewhat.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode550
Lib/test/test_compare.py:550: class Class_str(str):
Will rename to StrSubclass.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode554
Lib/test/test_compare.py:554: def some_func(self):
True. Will be removed.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode569
Lib/test/test_compare.py:569: self.assert_insts(c1, c1, True,   0,
Class_str.meth, Class_str.meth)
On the empty tuple meths: I wanted to have one place that has the
knowledge that there are no meths, instead of repeating that in every
invocation of assert_insts().

But I'm ok either way. Let me know which way you would recommend to go.

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode591
Lib/test/test_compare.py:591: i1 = int(10001)
Probably. But then would we also change other objects? As it is now, it
is at least consistent (across the test functions).

https://bugs.python.org/review/12067/diff/13975/Lib/test/test_compare.py#newcode881
Lib/test/test_compare.py:881: def
test__internal_is_value_comparable(self):
The idea was that "test_" is the common prefix for test functions, and
the extra underscore indicates an internal function. I do realize that
it is redundant with saying "internal" in the function name. However,
while trying to remove the second underscore, I realized something else:
The unittest package executes the test functions in alphabetical order
of their names. I think it makes sense to execute tests of internal
functions first, before they are used in the other real tests. Having
two underscores ensures that it is executed first. I will remove the
"internal_".

https://bugs.python.org/review/12067/


More information about the docs mailing list