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

[Enhancement] - Progressively change the code to be PEP8 and Flake8 compliant #476

Closed
studiosi opened this issue Jun 1, 2018 · 9 comments
Labels
medium priority refectoring/review Any areas of code that could be improved/refactored

Comments

@studiosi
Copy link

studiosi commented Jun 1, 2018

I think it would be great if the code would be PEP8 compliant.

In case that you don't know, the PEP8 is an official "style guide" from the Python team for Python code. PyCharm helps a lot with this, but I know it is a pain to go through all the files in the project doing small changes here and there. I just leave this idea for the future, especially if there is going to be a project-wide refactor. What do you think?

@alexlittle alexlittle added refectoring/review Any areas of code that could be improved/refactored medium priority labels Jun 4, 2018
@alexlittle
Copy link
Member

I just had a go with the pep8ify tool (http://pep8ify.com/) and applied on the oppia/gamification directory in the pep8ify branch (https://github.com/DigitalCampus/django-oppia/tree/pep8ify)

Seems that tool would make it quite easy to update all the code...

@studiosi
Copy link
Author

TBH most of the rules are easily mechanizable: one blank line between functions, spaces instead of tabs and so on... so I believe that these can be automatically changed. If we had a good test bench, we could be doing a whole project pep8ify and check automatically if something got broken.

@alexlittle
Copy link
Member

Can also use pycodestyle (http://pycodestyle.pycqa.org/en/latest/index.html) to help with this

alexlittle added a commit that referenced this issue Nov 3, 2019
alexlittle added a commit that referenced this issue Nov 3, 2019
alexlittle added a commit that referenced this issue Nov 3, 2019
alexlittle added a commit that referenced this issue Nov 3, 2019
alexlittle added a commit that referenced this issue Nov 3, 2019
alexlittle added a commit that referenced this issue Nov 3, 2019
alexlittle added a commit that referenced this issue Nov 3, 2019
alexlittle added a commit that referenced this issue Nov 3, 2019
alexlittle added a commit that referenced this issue Nov 3, 2019
@alexlittle
Copy link
Member

Remaining issues for this (with the exception of long line issues) are:

./oppia/models/main.py:54:9: E722 do not use bare 'except'
./oppia/models/main.py:187:9: E722 do not use bare 'except'
./oppia/models/main.py:236:9: E722 do not use bare 'except'
./oppia/models/main.py:248:9: E722 do not use bare 'except'
./oppia/models/main.py:269:13: E722 do not use bare 'except'
./oppia/models/main.py:414:9: E722 do not use bare 'except'
./oppia/models/main.py:426:9: E722 do not use bare 'except'
./quiz/api/serializers.py:54:21: E722 do not use bare 'except'
./quiz/api/serializers.py:69:17: E722 do not use bare 'except'
./helpers/templatetags/display_functions.py:44:5: E722 do not use bare 'except'
./helpers/templatetags/query_string.py:61:9: E722 do not use bare 'except'
./profile/views.py:497:13: E722 do not use bare 'except'
./api/resources.py:312:9: E722 do not use bare 'except'
./api/resources.py:599:9: E722 do not use bare 'except'
./viz/management/commands/ip2location.py:41:5: E722 do not use bare 'except'

./oppiamobile/settings.py:125:1: E731 do not assign a lambda expression, use a def

@studiosi
Copy link
Author

studiosi commented Nov 11, 2019

The bare except can be changed to a "except Exception" I believe that catches all the exceptions unless you want to catch other types of system-exiting exceptions (like KeyboardInterrupt).

@alexlittle
Copy link
Member

Thanks! I found a lot of these 'bare' excepts were around JSON parsing, so I was able to put in specific json encoding exceptions

alexlittle added a commit that referenced this issue Nov 13, 2019
alexlittle added a commit that referenced this issue Nov 13, 2019
alexlittle added a commit that referenced this issue Nov 14, 2019
alexlittle added a commit that referenced this issue Nov 14, 2019
alexlittle added a commit that referenced this issue Nov 14, 2019
alexlittle added a commit that referenced this issue Nov 14, 2019
alexlittle added a commit that referenced this issue Nov 15, 2019
@studiosi
Copy link
Author

Getting rid of pokemon exceptions is always good 💃

alexlittle added a commit that referenced this issue Nov 19, 2019
alexlittle added a commit that referenced this issue Nov 19, 2019
alexlittle added a commit that referenced this issue Nov 20, 2019
alexlittle added a commit that referenced this issue Nov 21, 2019
alexlittle added a commit that referenced this issue Nov 21, 2019
alexlittle added a commit that referenced this issue Nov 21, 2019
alexlittle added a commit that referenced this issue Nov 24, 2019
alexlittle added a commit that referenced this issue Nov 24, 2019
alexlittle added a commit that referenced this issue Nov 24, 2019
alexlittle added a commit that referenced this issue Nov 24, 2019
alexlittle added a commit that referenced this issue Nov 25, 2019
alexlittle added a commit that referenced this issue Nov 25, 2019
alexlittle added a commit that referenced this issue Nov 25, 2019
alexlittle added a commit that referenced this issue Nov 25, 2019
@alexlittle alexlittle changed the title [Enhancement] - Progressively change the code to be PEP8 compliant [Enhancement] - Progressively change the code to be PEP8 and Flake8 compliant Nov 26, 2019
@alexlittle
Copy link
Member

I updated this to include Flake8 too - from running this it picks up a lot of additional issues than pycodestyle does on it's own, eg. unused imports etc - see this file for the current list of Flake8 lint warnings
flake8.txt

alexlittle added a commit that referenced this issue Nov 26, 2019
alexlittle added a commit that referenced this issue Nov 26, 2019
alexlittle added a commit that referenced this issue Nov 26, 2019
alexlittle added a commit that referenced this issue Nov 26, 2019
alexlittle added a commit that referenced this issue Nov 26, 2019
alexlittle added a commit that referenced this issue Nov 27, 2019
@alexlittle
Copy link
Member

transferred to OPPIA-48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium priority refectoring/review Any areas of code that could be improved/refactored
Projects
None yet
Development

No branches or pull requests

2 participants