Py35 #29
Py35 #29
Conversation
| @@ -108,7 +108,12 @@ def get_consensus_stats(consensus_map): | |||
| cons = consensus_map.values() | |||
| total_cons = len(cons) | |||
|
|
|||
| rank_names = [c[0] for c in cons[0]] | |||
There was a problem hiding this comment.
@wasade please double check if this change still results in the intended logic
There was a problem hiding this comment.
because dict values are not indexable
t2t/nlevel.py
Outdated
| @@ -564,7 +563,7 @@ def make_consensus_tree(cons_split, check_for_rank=True, tips=None): | |||
| god_node = TreeNode(name=None) | |||
| god_node.Rank = None | |||
|
|
|||
| base = cons_split[0] | |||
| base = next(iter(cons_split)) | |||
There was a problem hiding this comment.
@wasade also here, I am not totally sure if the syntax change preserves your logic
There was a problem hiding this comment.
not sure but i think that's safe as long as the change works if cons_split is list or tuple or a generator
| obs = hash_cons(input, ['a', 'd', 'c', 'b'], 7) | ||
| self.assertTrue(array_equal(obs, exp)) | ||
| self.assertTrue(array_equal(obs.flatten(), exp)) |
There was a problem hiding this comment.
is there a better way?
|
@sjanssen2 Looks okay to me. Wait for @wasade 's more expertised review. |
| @@ -108,7 +108,12 @@ def get_consensus_stats(consensus_map): | |||
| cons = consensus_map.values() | |||
| total_cons = len(cons) | |||
|
|
|||
| rank_names = [c[0] for c in cons[0]] | |||
t2t/nlevel.py
Outdated
| @@ -564,7 +563,7 @@ def make_consensus_tree(cons_split, check_for_rank=True, tips=None): | |||
| god_node = TreeNode(name=None) | |||
| god_node.Rank = None | |||
|
|
|||
| base = cons_split[0] | |||
| base = next(iter(cons_split)) | |||
There was a problem hiding this comment.
not sure but i think that's safe as long as the change works if cons_split is list or tuple or a generator
| obs = hash_cons(input, ['a', 'd', 'c', 'b'], 7) | ||
| self.assertTrue(array_equal(obs, exp)) | ||
| self.assertTrue(array_equal(obs.flatten(), exp)) |
tests/test_consensus.py
Outdated
| @@ -178,8 +178,8 @@ def test_get_consensus_stats(self): | |||
| 's': set(['s__s1', 's__s2', 's__s3'])} | |||
| #exp_names = {'k':2,'p':1,'c':2,'o':1,'f':2,'g':2,'s':3} | |||
| obs_nseqs, obs_names = get_consensus_stats(input) | |||
| self.assertEqual(obs_nseqs, exp_nseqs) | |||
| self.assertEqual(obs_names, exp_names) | |||
| #self.assertEqual(obs_nseqs, exp_nseqs) | |||
There was a problem hiding this comment.
@wasade very good catch! because this test points out a conceptional problem.
In py35 the order of the dict key,value pairs seems to be different from the order in py27, i.e. they might come as:
['k__k2', '', 'c__c1', 'f__f2', 'f__f2', 'g__g2', 's__s3']
[None, None, None, None, None, None, None]
['k__k1', 'p__p1', 'c__c1', 'o__o1', 'f__f1', 'g__g1', 's__s1']
['k__k1', 'p__p1', 'c__c2', None, 'f__f2', 'g__g1', 's__s2']
which would result in ranks [k, "", c, f, f, g, s]. Do we have to assume this input is correct, even with 'f__f2', 'f__f2'? If yes, we need to change and maybe use a constant to define rank names? I don't see a schema to infer correct rank names from the above input :-/
Save fmeasures
|
Thanks!!! Do you think this is good to merge? |
With this PR the codebase gets migrated to python 3.x