lopq icon indicating copy to clipboard operation
lopq copied to clipboard

[MRG] Solve some pep8/flake8 issues & some code refactoring

Open mohamed-ali opened this issue 5 years ago • 3 comments

The purpose of this PR is to improve the code quality by reducing the number of alerts thrown by flake8:

$ flake8 . | wc -l 
     334

After the PR:

$ flake8 . | wc -l 
     235

The fourth commit reduces further the number of flake8 issues:

$ flake8 --exclude ./python/lopq/lopq_model_pb2.py . | wc -l 
     155

With commit 5-> 7 the number of issues is further reduced:

$ flake8 --exclude ./python/lopq/lopq_model_pb2.py . | wc -l 
     112

Down to 57 with the commit 8:

$ flake8 --exclude=./python/lopq/lopq_model_pb2.py .  | wc -l 
      57

The commits 9 and 10 finish fixing all identified flake8 issues:

$ flake8 --exclude=./python/lopq/lopq_model_pb2.py . | wc -l 
       0

PS: lopq_model_pb2.py is skipped because it's autogenerated by protobuf

mohamed-ali avatar Apr 10 '19 19:04 mohamed-ali

Coverage Status

Coverage remained the same at 88.443% when pulling 031d4b657a93263c4d0672af672882e871b699fc on mohamed-ali:fix-some-flake8-issues into 0f17655b901e6dfabe5c2aa62b4c8e492f34b05a on yahoo:master.

coveralls avatar Apr 10 '19 20:04 coveralls

Thanks a lot for this! It looks excellent. Actually I am no longer an admin of this repo since I no longer work at Yahoo, so I currently cannot do the merge. However, I am working out an arrangement so that I can continue to maintain this. I'll follow up here soon with an update and we can get this and your other PR merged. Thanks.

pumpikano avatar Apr 12 '19 23:04 pumpikano

Hi @pumpikano, is there any progress on the status/maintainability of this library? Thanks.

mohamed-ali avatar Jul 02 '19 13:07 mohamed-ali