From 58ad8870108525770a064ff72e2ff2c529d8ba83 Mon Sep 17 00:00:00 2001 From: Tim Littlefair Date: Fri, 9 Mar 2018 22:20:29 +0800 Subject: [PATCH 1/3] Added test class for issue 28, which runs OK under Python2.7 but throws an exception under Python3.5 (both on macOS). As well as adding the test class, this commit turns on DEBUG level logging, which makes it easier to spot the point at which the behaviour of the two versions diverge. --- cluster/test/test_hierarchical.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/cluster/test/test_hierarchical.py b/cluster/test/test_hierarchical.py index 519cb47..8ee62da 100644 --- a/cluster/test/test_hierarchical.py +++ b/cluster/test/test_hierarchical.py @@ -231,12 +231,42 @@ def euclidian_distance(a, b): result = cl.getlevel(40) self.assertIsNotNone(result) +class Issue28TestCase(Py23TestCase): + ''' + Test case to cover the case where the data consist + of dictionary keys, and the distance function executes + on the values these keys are associated with in the + dictionary, rather than the keys themselves. + + Behaviour for this test case differs between Python2.7 + and Python3.5: on 2.7 the test behaves as expected, + + See Github issue #28. + ''' + + def testIssue28(self): + "Issue28 (Hierarchical Clustering)" + points1D = { + 'p4' : 5, 'p2' : 6, 'p7' : 10, + 'p9' : 120, 'p10' : 121, 'p11' : 119, + } + + distance_func = lambda a,b : abs(points1D[a]-points1D[b]) + cl = HierarchicalClustering(list(points1D.keys()), distance_func) + result = cl.getlevel(20) + self.assertIsNotNone(result) + if __name__ == '__main__': + + import logging + suite = unittest.TestSuite(( unittest.makeSuite(HClusterIntegerTestCase), unittest.makeSuite(HClusterSmallListTestCase), unittest.makeSuite(HClusterStringTestCase), + unittest.makeSuite(Issue28TestCase), )) + logging.basicConfig(level=logging.DEBUG) unittest.TextTestRunner(verbosity=2).run(suite) From 28db3164b42516ff0f62b7798ce612c24a5c921f Mon Sep 17 00:00:00 2001 From: Tim Littlefair Date: Fri, 9 Mar 2018 22:43:18 +0800 Subject: [PATCH 2/3] Investigation of GitHub issue 28 suggests that it relates in some way to the logic linkage.py. This change ensures that the tests which have been written in this file can be run from the CLI. The tests presently run successfully under both Python2.7 and Python3.5. --- cluster/test/test_linkage.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cluster/test/test_linkage.py b/cluster/test/test_linkage.py index 0a5df07..e84b492 100644 --- a/cluster/test/test_linkage.py +++ b/cluster/test/test_linkage.py @@ -29,3 +29,14 @@ def test_average_distance(self): result = average(self.set_a, self.set_b, self.dist) expected = 22.5 self.assertEqual(result, expected) + +if __name__ == '__main__': + + import logging + + suite = unittest.TestSuite(( + unittest.makeSuite(LinkageMethods), + )) + + logging.basicConfig(level=logging.DEBUG) + unittest.TextTestRunner(verbosity=2).run(suite) From f77df2d7a077611dc7724d9cbc0edcf632722760 Mon Sep 17 00:00:00 2001 From: Tim Littlefair Date: Fri, 9 Mar 2018 23:00:40 +0800 Subject: [PATCH 3/3] Replaced two lines repeated twice in function genmatrix with a function call, which is intended to be identical in effect, but provides an informative log message. With this commit, the behaviour of the test_hierarchical.py test set remains identical to python-cluster 1.4.1 under both Python2.7 (which passes)_and Python3.5 (which fails with a KeyError thrown). --- cluster/matrix.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/cluster/matrix.py b/cluster/matrix.py index 8ecf287..e9c77af 100644 --- a/cluster/matrix.py +++ b/cluster/matrix.py @@ -22,6 +22,30 @@ logger = logging.getLogger(__name__) +def _encapsulate_item_for_combinfunc(item): + """ + This function has been extracted in order to + make Github issue #28 easier to investigate. + It replaces the following two lines of code, + which occur twice in method genmatrix, just + before the invocation of combinfunc. + if not hasattr(item, '__iter__') or isinstance(item, tuple): + item = [item] + Logging has been added to the original two lines + and shows that the behaviour of this snippet + has changed between Python2.7 and Python3.5. + """ + encapsulated_item = None + if not hasattr(item, '__iter__') or isinstance(item, tuple): + encapsulated_item = [item] + else: + encapsulated_item = item + logging.debug( + "item class:%s encapsulated as:%s ", + item.__class__, encapsulated_item.__class__ + ) + return encapsulated_item + class Matrix(object): """ @@ -123,10 +147,17 @@ def genmatrix(self, num_processes=1): num_tasks_completed += 1 else: # Otherwise do it here, in line + """ if not hasattr(item, '__iter__') or isinstance(item, tuple): item = [item] if not hasattr(item2, '__iter__') or isinstance(item2, tuple): item2 = [item2] + """ + # See the comment in function _encapsulate_item_for_combinfunc + # for details of why the lines above have been replaced + # by function invocations + item = _encapsulate_item_for_combinfunc(item) + item2 = _encapsulate_item_for_combinfunc(item2) row[col_index] = self.combinfunc(item, item2) if self.symmetric: