Remove unnecessary creations of lists at all #2261
Conversation
horpto
force-pushed
the
extra-copy-list
branch
from
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.
gensim/models/ldamodel.py
Outdated
| @@ -567,15 +567,18 @@ def init_dir_prior(self, prior, name): | |||
| if isinstance(prior, six.string_types): | |||
| if prior == 'symmetric': | |||
| 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.
| @@ -674,7 +674,6 @@ def online_sanity(self, model): | |||
| 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 |
horpto
changed the title
- hanged indent - return assertTrue line - fix bug in _get_average_score in python2 - a bit refactor bm25
horpto
force-pushed
the
extra-copy-list
branch
from
2a03366 to
14339f2
Compare
horpto
changed the title
menshikh-iv
changed the title
| @@ -567,15 +567,18 @@ def init_dir_prior(self, prior, name): | |||
| if isinstance(prior, six.string_types): | |||
| if prior == 'symmetric': | |||
| 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.