Random sample on stream #1444
Random sample on stream #1444
Conversation
gensim/corpora/textcorpus.py
Outdated
| for i, sample in enumerate(self.get_texts()): | ||
| seen_documents = i + 1 |
There was a problem hiding this comment.
What is the purpose of introducing this new variable?
There was a problem hiding this comment.
Oh I think I see -- you can directly re-use the counter i from enumerate (just give it a better name, such as seen_documents).
There was a problem hiding this comment.
It may be only my personal opinion, but I do not like this type of "variable leaking" out of the for loop scope.
There was a problem hiding this comment.
OK, why not. Then I suggest seen_documents += 1 at least. This pattern is too obscure.
|
Ping @vlejd, when you finish your PR? |
|
Thank you @vlejd |
gensim/corpora/textcorpus.py
Outdated
| @@ -305,7 +302,7 @@ def sample_texts(self, n, seed=None, length=None): | |||
| # This means that length was set to be greater than number of items in corpus | |||
| # and we were not able to sample enough documents before the stream ended. | |||
| raise ValueError( | |||
| "length {0:d} greater than number of documents in corpus {1:d}".format(length, seen_documents)) | |||
| "length {0:d} greater than number of documents in corpus {1:d}".format(length, i + 1)) | |||
There was a problem hiding this comment.
This will fail with an exception if there are no texts (i will be undefined).
There was a problem hiding this comment.
i undefined if only self.getstream() is empty
Related to #308.
Fix comments in #1422.