Skip to content
This repository has been archived by the owner on Jan 9, 2024. It is now read-only.

Lint Python code for syntax errors #4

Merged
merged 13 commits into from
Sep 11, 2020
Merged

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Jul 12, 2020

Demonstrating why we need #3 or similar. Five files with Python syntax errors.

Output: https://github.com/cclauss/sobot-rimulator/actions

flake8 testing of https://github.com/nmccrea/sobot-rimulator on Python 3.8.3

$ flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics

./models/supervisor_state_machine.py:180:11: E999 SyntaxError: invalid syntax
    print "\n ======== \n"
          ^
./models/controllers/gtg_and_ao_controller.py:106:11: E999 SyntaxError: invalid syntax
    print "\n\n"
          ^
./models/controllers/go_to_goal_controller.py:92:11: E999 SyntaxError: invalid syntax
    print "\n\n"
          ^
./models/controllers/follow_wall_controller.py:182:11: E999 SyntaxError: invalid syntax
    print "\n\n"
          ^
./models/controllers/avoid_obstacles_controller.py:114:11: E999 SyntaxError: invalid syntax
    print "\n\n"
          ^
5     E999 SyntaxError: invalid syntax
5

https://flake8.pycqa.org/en/latest/user/error-codes.html

On the flake8 test selection, this PR does not focus on "style violations" (the majority of flake8 error codes that psf/black can autocorrect). Instead these tests are focus on runtime safety and correctness:

  • E9 tests are about Python syntax errors usually raised because flake8 can not build an Abstract Syntax Tree (AST). Often these issues are a sign of unused code or code that has not been ported to Python 3. These would be compile-time errors in a compiled language but in a dynamic language like Python they result in the script halting/crashing on the user.
  • F63 tests are usually about the confusion between identity and equality in Python. Use ==/!= to compare str, bytes, and int literals is the classic case. These are areas where a == b is True but a is b is False (or vice versa). Python >= 3.8 will raise SyntaxWarnings on these instances.
  • F7 tests logic errors and syntax errors in type hints
  • F82 tests are almost always undefined names which are usually a sign of a typo, missing imports, or code that has not been ported to Python 3. These also would be compile-time errors in a compiled language but in Python a NameError is raised which will halt/crash the script on the user.

Demonstrating why we need nmccrea#3 or similar.
@nmccrea
Copy link
Owner

nmccrea commented Jul 15, 2020

@cclauss thanks so much for this. This project has been in suspended animation for some time but I will be getting things moving again in short order.

@nmccrea nmccrea self-assigned this Sep 11, 2020
@nmccrea nmccrea self-requested a review September 11, 2020 16:48
@nmccrea nmccrea changed the base branch from master to pr-fix-target September 11, 2020 17:23
@nmccrea nmccrea changed the base branch from pr-fix-target to master September 11, 2020 17:23
nmccrea
nmccrea previously approved these changes Sep 11, 2020
nmccrea
nmccrea previously approved these changes Sep 11, 2020
@nmccrea nmccrea merged commit 4781de8 into nmccrea:master Sep 11, 2020
@nmccrea
Copy link
Owner

nmccrea commented Sep 11, 2020

@cclauss Thank you very much for this. I have now linted this entire project with black, codespell, and flake8. I made some modifications to the Github action you created here but the spirit remains the same.

Among other things I have made the action fail if any of these three tools find problems. At the moment, these checks still do not pass. This is because there are a handful of remaining problems found by black and flake8. They fall into two categories:

  • module imports - The use of GTK and the associated gi library requires some funky import behavior. I am hoping to migrate this project away from GTK at some point soon so that should solve these problems.
  • line length - Most remaining line length errors can be resolved by cleaning up the code and implementation - editing comments, shortening names, and realizing readability improvements that come with better separation of concerns.

My hope is that we can get these remaining problems cleaned up before long and then begin enforcing this action for all PRs.

@cclauss cclauss deleted the patch-1 branch September 11, 2020 18:20
@nmccrea nmccrea mentioned this pull request Oct 18, 2020
12 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants