Reformat and lint files with black and flake8 #168
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
September 4, 2020 21:12
Format with black; fix many, many defects identified with pylint
Python prefers snake_case for script and module names and this fixes pylint complaining about this
black target will reformat all Python scripts flake8 will run linter on all Python scripts
Passes flake8 (required) and most of pylint (recommended)
Pass flake8 (required) and most of pylint (recommended)
black should be used for formatting scripts must be error free with flake8
graeme-a-stewart
added
the
work in progress
pylint checks pushed an implementation where most code was inside the main() function, however this caused a problem with unittest that will only run tests in the global namespace, but use of globals is rather nasty This reimplementaion keeps the argument parsing and test case generation in a main-ish function, but returns the test case and then invokes unittest.main() which is a much nicer implementation
Only need to run flake8 on one platform
The urlopen() call in these tests is to the http server that was setup by the unittest itself, so we know that it's ok
This was an accident, but it proved that the flake8 GH action works as we want!
graeme-a-stewart
added
enhancement
|
Hi @amete - this should be good to go now. All the scripts have been reformatted and small translation bugs are fixed |
amete
approved these changes
Sep 7, 2020
There was a problem hiding this comment.
Thanks a lot for this @graeme-a-stewart! The python code looks quite a bit cleaner now. All tests pass (locally as well). I ran the plotting script over a test file and the output looks good (i.e. no change). I'm merging now.
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A bit of an overdue clean-up of all of the Python scripts.
blackflake8The scripts were also checked against
pylint, and many issues from there fixed. However, it's exceedling strict and there are even some formatting decisions wherepylintandblackdisagree, so that without specific exceptions added to the scripts the two checks can't agree. Most of the scripts test at 9/10 (or better) withpylint. We should endevour to keep the scripts good againstpylintwithout enforcing it.A GitHub action was added to run
flake8as part of the CI.Fixes #157