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

[MRG] PEP8 #57

Merged
merged 3 commits into from
Jun 18, 2016
Merged

[MRG] PEP8 #57

merged 3 commits into from
Jun 18, 2016

Conversation

takuti
Copy link
Contributor

@takuti takuti commented Jun 15, 2016

As you know, Python has an official coding style guide PEP8. In order to follow PEP8, I have fixed several lines of code under the fastFM/ folder.

$ pep8 fastFM                                                                                                                                                                     
fastFM/mcmc.py:12:19: E128 continuation line under-indented for visual indent
fastFM/validation.py:100:13: W503 line break before binary operator
fastFM/validation.py:165:80: E501 line too long (85 > 79 characters)
fastFM/tests/test_als.py:79:25: E231 missing whitespace after ','
fastFM/tests/test_als.py:81:33: E231 missing whitespace after ','
fastFM/tests/test_als.py:88:13: E128 continuation line under-indented for visual indent
fastFM/tests/test_als.py:94:16: E231 missing whitespace after ','
fastFM/tests/test_als.py:143:13: E128 continuation line under-indented for visual indent
fastFM/tests/test_als.py:151:80: E501 line too long (84 > 79 characters)
fastFM/tests/test_als.py:152:80: E501 line too long (81 > 79 characters)
fastFM/tests/test_als.py:160:17: E128 continuation line under-indented for visual indent
fastFM/tests/test_als.py:162:80: E501 line too long (84 > 79 characters)
fastFM/tests/test_als.py:163:80: E501 line too long (87 > 79 characters)
fastFM/tests/test_als.py:170:5: E265 block comment should start with '# '
fastFM/tests/test_base.py:32:13: E128 continuation line under-indented for visual indent
fastFM/tests/test_datasets.py:17:80: E501 line too long (83 > 79 characters)
fastFM/tests/test_datasets.py:26:80: E501 line too long (89 > 79 characters)
fastFM/tests/test_ffm.py:10:1: E302 expected 2 blank lines, found 1
fastFM/tests/test_ffm.py:23:1: E302 expected 2 blank lines, found 1
fastFM/tests/test_ffm.py:24:64: E231 missing whitespace after ','
fastFM/tests/test_ffm.py:27:1: E302 expected 2 blank lines, found 1
fastFM/tests/test_mcmc.py:52:29: E231 missing whitespace after ','
fastFM/tests/test_mcmc.py:67:29: E231 missing whitespace after ','
fastFM/tests/test_mcmc.py:117:13: E128 continuation line under-indented for visual indent
fastFM/tests/test_mcmc.py:119:9: E128 continuation line under-indented for visual indent
fastFM/tests/test_mcmc.py:121:80: E501 line too long (87 > 79 characters)
fastFM/tests/test_mcmc.py:122:13: E128 continuation line under-indented for visual indent
fastFM/tests/test_ranking.py:27:1: E302 expected 2 blank lines, found 1
fastFM/tests/test_ranking.py:46:13: E128 continuation line under-indented for visual indent
fastFM/tests/test_ranking.py:47:13: E128 continuation line under-indented for visual indent
fastFM/tests/test_ranking.py:55:1: W391 blank line at end of file
fastFM/tests/test_sgd.py:34:13: E128 continuation line under-indented for visual indent
fastFM/tests/test_sgd.py:35:13: E128 continuation line under-indented for visual indent
fastFM/tests/test_sgd.py:48:13: E128 continuation line under-indented for visual indent
fastFM/tests/test_sgd.py:55:12: E225 missing whitespace around operator
fastFM/tests/test_utils.py:7:1: E302 expected 2 blank lines, found 1
fastFM/tests/test_utils.py:13:44: E222 multiple spaces after operator

@takuti
Copy link
Contributor Author

takuti commented Jun 15, 2016

Note that flake8 tells us meaningless lines as:

$ flake8 fastFM
fastFM/mcmc.py:9:1: F811 redefinition of unused 'assert_all_finite' from line 8
fastFM/tests/test_als.py:11:1: F811 redefinition of unused 'assert_almost_equal' from line 8
fastFM/tests/test_ffm.py:6:1: F401 'mean_squared_error' imported but unused
fastFM/tests/test_ffm.py:6:1: F401 'r2_score' imported but unused

Can I also fix the above notices in this PR?

@takuti
Copy link
Contributor Author

takuti commented Jun 16, 2016

Since I'm actively tracking fastFM code, I have also noticed some whitespace-related PEP violations as b7fba9c.

Actually, flake8 and pep8 do not support checking Cython code, but several OSS communities try to check and fix them as pandas-dev/pandas#12995.

@ibayer
Copy link
Owner

ibayer commented Jun 17, 2016

@takuti thanks for you effort.
I'm a bit embarrassed to see so many pep8 violations ;-) .

Good to know this:
flake8 ffm.pyx --select=E501,E302,E203,E226,E111,E114,E221,E303,E128,E231,E126,E128

Please feel free to remove unused lines.
Let me know when you think this PR should be merged or add [MRG] to the title.

@takuti
Copy link
Contributor Author

takuti commented Jun 18, 2016

@ibayer Thanks for your response :)

In fact, scikit-learn shows numerous PEP8 violations as a result of $ pep8 sklearn, so following PEP8 strictly on actively developed OSS projects seems to be hard problem 😞

For the unused lines, I have removed them. You can merge now.

@takuti takuti changed the title PEP8 [MRG] PEP8 Jun 18, 2016
@ibayer ibayer merged commit 919fb71 into ibayer:master Jun 18, 2016
@ibayer
Copy link
Owner

ibayer commented Jun 18, 2016

@takuti thanks for the clean up.

@takuti takuti deleted the pep8 branch June 29, 2016 07:28
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

Successfully merging this pull request may close these issues.

2 participants