Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typos, text and code fix in LDA tutorial #3289

Merged
merged 4 commits into from
Mar 22, 2022
Merged

Typos, text and code fix in LDA tutorial #3289

merged 4 commits into from
Mar 22, 2022

Conversation

davebulaval
Copy link
Contributor

This PR fixes the following in all three files format:

  • unused import (os.path and io)
  • Fix some dead code
  • Propose some text improvements

Moreover, the following link is broken http://rare-technologies.com/lda-training-tips/, and I was not able to find the new URL.

image

@piskvorky piskvorky changed the title typos, text and code fix Typos, text and code fix in LDA tutorial Feb 18, 2022
@piskvorky piskvorky added this to the Next release milestone Feb 18, 2022
@piskvorky piskvorky added bug Issue described a bug documentation Current issue related to documentation labels Feb 18, 2022
@piskvorky
Copy link
Owner

piskvorky commented Feb 18, 2022

Is this a part of some assignment?

@davebulaval
Copy link
Contributor Author

Is this a part of some assignment?

@piskvorky, no, I was just reading the tutorial and found some errors. Just a free contribution.

@davebulaval
Copy link
Contributor Author

davebulaval commented Feb 18, 2022

@piskvorky
Also, after trying different things, reading many Gensim tutorials trying to find the keywords (because I do not know all the doc elements of Gensim nor have time to do it or use logger in my ML routines), I cannot find the response to that sentence:

First, enable logging (as described in many Gensim tutorials), and set eval_every = 1 in LdaModel

I've tried Googling it using the doc search bar (that is a never-ending search BTW) and could not find a working example in less time than it would require to just describe the complete solution in the tutorial.
(Sorry if my text sound a little bit harsh, I'm just pissed to have lost 10 minutes trying to add something so simple)

So I propose to simply add the proper way to set the logger (logger.setLevel(logging.DEBUG)).

@piskvorky
Copy link
Owner

piskvorky commented Feb 18, 2022

Yeah, that phrase as described in many Gensim tutorials in unnecessarily cryptic.

Can you include this code in its place instead?

logging.basicConfig(level=logging.INFO, format='PID:%(process)d:%(threadName)s - %(asctime)s - %(levelname)s - %(filename)s:%(lineno)s - %(message)s')

Thanks!

@davebulaval
Copy link
Contributor Author

Yeah, that phrase as described in many Gensim tutorials in unnecessarily cryptic.

Can you include this code in its place instead?

logging.basicConfig(level=logging.INFO, format='PID:%(process)d:%(threadName)s - %(asctime)s - %(levelname)s - %(filename)s:%(lineno)s - %(message)s')

Thanks!

Fixed in 00b54dc

@piskvorky
Copy link
Owner

LGTM, thanks! The failing wheels seem unrelated, @mpenkov is working on those.

@davebulaval
Copy link
Contributor Author

@piskvorky glad to help!

@codecov
Copy link

codecov bot commented Mar 18, 2022

Codecov Report

Merging #3289 (f3e1671) into develop (a936521) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #3289   +/-   ##
========================================
  Coverage    79.53%   79.53%           
========================================
  Files           68       68           
  Lines        11781    11783    +2     
========================================
+ Hits          9370     9372    +2     
  Misses        2411     2411           
Impacted Files Coverage Δ
gensim/models/keyedvectors.py 82.68% <0.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a936521...f3e1671. Read the comment docs.

@mpenkov mpenkov merged commit 4bfb777 into piskvorky:develop Mar 22, 2022
@mpenkov
Copy link
Collaborator

mpenkov commented Mar 22, 2022

Merged, thanks @davebulaval !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue described a bug documentation Current issue related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants