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

comply to PEP8 #10

Closed
piskvorky opened this issue Mar 4, 2011 · 8 comments
Closed

comply to PEP8 #10

piskvorky opened this issue Mar 4, 2011 · 8 comments
Assignees

Comments

@piskvorky
Copy link
Owner

Change camelCase vars and function names and parameters to python_style. This will break backward compatibility, so it best be done all at once (rather than incrementally).

http://groups.google.com/group/gensim/browse_thread/thread/73f4301723cea765

@piskvorky
Copy link
Owner Author

Scheduled for 0.8.0.

I'll probably add some wrappers to print deprecated warning when using the old names, together with hint how to fix it. To be completely removed in the next version (prolly 0.8.1).

@ghost ghost assigned piskvorky Apr 24, 2011
@piskvorky
Copy link
Owner Author

Changed all function/variable names, commit 361003c3772cba677a80a57e0a7c3e648f42f6e0 .

There is no deprecated warnings, the changes weren't that many plus they're very straightforward (numTopics=>num_topics, addDocuments=>add_documents etc.).

@piskvorky piskvorky reopened this Jun 9, 2011
@piskvorky
Copy link
Owner Author

reopening (forgot to change and test documentation as well :F )

piskvorky added a commit that referenced this issue Jun 9, 2011
* backwards incompatible, breaks all existing code!
* but the changes are straightforward: numTopics => num_topics, addDocuments => add_documents etc.
* documentation to be updated in a separate commit
@piskvorky
Copy link
Owner Author

Documentation updated in commit 88f2a3b .

All of gensim is now proper PEP8!

@mcobzarenco
Copy link
Contributor

This is not the case anymore, (e.g. pyflake shows 63 warnings for models/wrappers/fasttext.py). Happy to PEP8 the whole repo and create a pr if that's cool.

@piskvorky
Copy link
Owner Author

@mcobzarenco what warnings are these? For example, we care more about readability than a strict 80-char-line limit.

But I also noticed some lapses (vertical indent in word2vec). @menshikh-iv what's the status on code style consistency in gensim?

@menshikh-iv
Copy link
Contributor

menshikh-iv commented Oct 19, 2017

@piskvorky

(math) ivan@P50:~/release/gensim$ git log | head -n5
commit e92b45d3f83ad670da9cf6ae20ae86a2a1c8558c
Author: jodevak <jodevak@users.noreply.github.com>
Date:   Thu Oct 19 09:37:41 2017 +0300

    Add build_vocab_from_freq to Word2Vec, speedup scan_vocab (#1599)
(math) ivan@P50:~/release/gensim$ find gensim/ -name "*.py" | xargs flake8 --ignore=E501,E731,E12,W503,E402 --show-source
gensim/test/test_lsimodel.py:95:5: E303 too many blank lines (2)
    def testCorpusTransform(self):
    ^

@mcobzarenco
Copy link
Contributor

Right, fair enough, all of the warnings were of the types you ignore above. Do you think we could we add those ignored rules to setup.cfg so they're documented and also ignored by default? I created a PR here #1636

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants