Remove unnecessary creations of lists at all#2261
Conversation
0e36ad5 to
2a03366
Compare
There was a problem hiding this comment.
Beautiful! This set of changes fits very well with Gensim mission and philosophy, thank you.
I remember some constructs have a problem with empty generator, but accept empty list. This is a corner case we have to be the most careful, with these changes. I hope the tests cover it well.
| logger.info("using symmetric %s at %s", name, 1.0 / self.num_topics) | ||
| init_prior = np.asarray([1.0 / self.num_topics for i in xrange(prior_shape)], dtype=self.dtype) | ||
| init_prior = np.fromiter((1.0 / self.num_topics for i in xrange(prior_shape)), | ||
| dtype=self.dtype, count=prior_shape) |
There was a problem hiding this comment.
Hanging indent please, here and elsewhere.
We avoid vertical indent because it leads to inconsistent visual indenting (like here), especially when the lines are edited in the future.
| terro.append(l) | ||
| else: | ||
| others.append(l) | ||
| self.assertTrue(all(['terrorism' not in l for l in others])) |
There was a problem hiding this comment.
I thought that this line does not check anything worthwhile. By code it is clear that this check is always true. It's not critical for me, so I return this line back.
|
ping @horpto, please fix @piskvorky comments and I'll merge current PR (don't forget to merge fresh |
- hanged indent - return assertTrue line - fix bug in _get_average_score in python2 - a bit refactor bm25
2a03366 to
14339f2
Compare
| logger.info("using symmetric %s at %s", name, 1.0 / self.num_topics) | ||
| init_prior = np.asarray([1.0 / self.num_topics for i in xrange(prior_shape)], dtype=self.dtype) | ||
| init_prior = np.fromiter((1.0 / self.num_topics for i in xrange(prior_shape)), | ||
| dtype=self.dtype, count=prior_shape) |
There was a problem hiding this comment.
This is still vertical indent.
In gensim, we use hanging indent always, consistently: https://www.python.org/dev/peps/pep-0008/#indentation
No description provided.