[chirp_devel] [PATCH 0/3] pep8 improvements
These patches improve CHIRP's pep8 checking. The first makes the cpep8.py script work on Windows by eliminating the dynamic manifest generation in favor of a static file. The second patch factors the virtualenv out of run_all_tests.sh into a new cpep8.sh script. Finally, the third patch bumps the pep8 requirement from 1.4.6 to 1.6.2 and chases down the new style issues that it caused.
Zach Welch (3): Improve cpep8.py (#2355) Add cpep8.sh wrapper script (#2355) Update cpep8 to use pep8 1.6.2 (#2355)
.hgignore | 3 +- chirp/generic_xml.py | 2 +- chirp/platform.py | 14 ++++-- chirp/template.py | 8 +-- chirp/xml_ll.py | 26 +++++----- chirpc | 7 +-- chirpui/clone.py | 2 +- chirpw | 16 +++--- run_all_tests.sh | 24 +-------- setup.py | 4 +- tools/cpep8.manifest | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++ tools/cpep8.py | 39 +++++++++++---- tools/cpep8.sh | 21 ++++++++ 13 files changed, 231 insertions(+), 74 deletions(-) create mode 100755 tools/cpep8.sh
# HG changeset patch # User Zach Welch zach@mandolincreekfarm.com # Fake Node ID fdb1083cf35cf98f316a35006db2cc335099a18f
Improve cpep8.py (#2355)
This patch expands cpep8.manifest to contain all files, allowing the cpep8.py script to use it instead of scanning for files. Scanning can now be done with --scan. If both --scan and --update options are given, the manifest file will be updated with the scan results.
Altogether, this should fix problems that Windows users were seeing, caused by mismatches between the dynamic manifest and the blacklist (due to different path separators).
diff --git a/tools/cpep8.manifest b/tools/cpep8.manifest index 2fe9d77..dbd90be 100644 --- a/tools/cpep8.manifest +++ b/tools/cpep8.manifest @@ -1,4 +1,143 @@ +./chirp/__init__.py +./chirp/alinco.py +./chirp/anytone.py +./chirp/ap510.py +./chirp/bandplan.py +./chirp/bandplan_au.py +./chirp/bandplan_iaru_r1.py +./chirp/bandplan_iaru_r2.py +./chirp/bandplan_iaru_r3.py +./chirp/bandplan_na.py +./chirp/baofeng_uv3r.py +./chirp/bitwise.py +./chirp/bitwise_grammar.py +./chirp/bjuv55.py +./chirp/chirp_common.py +./chirp/detect.py +./chirp/directory.py +./chirp/elib_intl.py +./chirp/errors.py +./chirp/ft1802.py +./chirp/ft1d.py +./chirp/ft2800.py +./chirp/ft50.py +./chirp/ft50_ll.py +./chirp/ft60.py +./chirp/ft7800.py +./chirp/ft817.py +./chirp/ft857.py +./chirp/ft90.py +./chirp/ftm350.py +./chirp/generic_csv.py +./chirp/generic_tpe.py +./chirp/generic_xml.py +./chirp/h777.py +./chirp/ic208.py +./chirp/ic2100.py +./chirp/ic2200.py +./chirp/ic2720.py +./chirp/ic2820.py +./chirp/ic9x.py +./chirp/ic9x_icf.py +./chirp/ic9x_icf_ll.py +./chirp/ic9x_ll.py +./chirp/icf.py +./chirp/icomciv.py +./chirp/icq7.py +./chirp/ict70.py +./chirp/ict7h.py +./chirp/ict8.py +./chirp/icw32.py +./chirp/icx8x.py +./chirp/icx8x_ll.py +./chirp/id31.py +./chirp/id51.py +./chirp/id800.py +./chirp/id880.py +./chirp/idrp.py +./chirp/import_logic.py +./chirp/kenwood_hmk.py +./chirp/kenwood_itm.py +./chirp/kenwood_live.py +./chirp/kguv8d.py +./chirp/kyd.py +./chirp/leixen.py +./chirp/logger.py +./chirp/memmap.py +./chirp/platform.py +./chirp/puxing.py +./chirp/pyPEG.py +./chirp/radioreference.py +./chirp/rfinder.py +./chirp/settings.py +./chirp/template.py +./chirp/th9800.py +./chirp/th_uv3r.py +./chirp/th_uv3r25.py +./chirp/th_uvf8d.py +./chirp/thd72.py +./chirp/thuv1f.py +./chirp/tk8102.py +./chirp/tmv71.py +./chirp/tmv71_ll.py +./chirp/util.py +./chirp/uv5r.py +./chirp/uvb5.py +./chirp/vx170.py +./chirp/vx2.py +./chirp/vx3.py +./chirp/vx5.py +./chirp/vx510.py +./chirp/vx6.py +./chirp/vx7.py +./chirp/vx8.py +./chirp/vxa700.py +./chirp/wouxun.py +./chirp/wouxun_common.py +./chirp/xml_ll.py +./chirp/yaesu_clone.py ./chirpc +./chirpui/__init__.py +./chirpui/bandplans.py +./chirpui/bankedit.py +./chirpui/clone.py +./chirpui/cloneprog.py +./chirpui/common.py +./chirpui/config.py +./chirpui/dstaredit.py +./chirpui/editorset.py +./chirpui/fips.py +./chirpui/importdialog.py +./chirpui/inputdialog.py +./chirpui/mainapp.py +./chirpui/memdetail.py +./chirpui/memedit.py +./chirpui/miscwidgets.py +./chirpui/radiobrowser.py +./chirpui/reporting.py +./chirpui/settingsedit.py +./chirpui/shiftdialog.py ./chirpw +./csvdump.py +./csvdump/__init__.py +./csvdump/csvapp.py +./csvdump/csvdump.py +./locale/check_parameters.py ./rpttool +./setup.py +./share/make_supported.py +./tests/__init__.py ./tests/run_tests +./tests/unit/__init__.py +./tests/unit/base.py +./tests/unit/test_bitwise.py +./tests/unit/test_chirp_common.py +./tests/unit/test_import_logic.py +./tests/unit/test_mappingmodel.py +./tests/unit/test_memedit_edits.py +./tests/unit/test_platform.py +./tests/unit/test_settings.py +./tests/unit/test_shiftdialog.py +./tools/bitdiff.py +./tools/cpep8.py +./tools/img2thd72.py diff --git a/tools/cpep8.py b/tools/cpep8.py index efaca7a..db33ab1 100755 --- a/tools/cpep8.py +++ b/tools/cpep8.py @@ -23,8 +23,6 @@ import logging import argparse import pep8
-scriptdir = os.path.dirname(sys.argv[0]) - parser = argparse.ArgumentParser() parser.add_argument("-a", "--all", action="store_true", help="Check all files, ignoring blacklist") @@ -32,8 +30,10 @@ parser.add_argument("-d", "--dir", action="store", default=".", help="Root directory of source tree") parser.add_argument("-s", "--stats", action="store_true", help="Only show statistics") +parser.add_argument("-S", "--scan", action="store_true", + help="Scan for additional files") parser.add_argument("-u", "--update", action="store_true", - help="Update the blacklist file") + help="Update manifest/blacklist files") parser.add_argument("-v", "--verbose", action="store_true", help="Display list of checked files") parser.add_argument("files", metavar="file", nargs='*', @@ -48,16 +48,25 @@ def file_to_lines(name): fh.close() return lines
-# read manifest and search for python files -manifest = file_to_lines(os.path.join(scriptdir, "cpep8.manifest")) -for root, dirs, files in os.walk(args.dir): - for f in files: - if f.endswith('.py'): - manifest.append(os.path.join(root, f))
-# read the blacklisted source files +scriptdir = os.path.dirname(sys.argv[0]) +manifest_filename = os.path.join(scriptdir, "cpep8.manifest") blacklist_filename = os.path.join(scriptdir, "cpep8.blacklist") -blacklist = file_to_lines(blacklist_filename) + +manifest = [] +if args.scan: + for root, dirs, files in os.walk(args.dir): + for f in files: + filename = os.path.join(root, f) + if f.endswith('.py'): + manifest.append(filename) + continue + with file(filename, "r") as fh: + shebang = fh.readline() + if shebang.startswith("#!/usr/bin/env python"): + manifest.append(filename) +else: + manifest += file_to_lines(manifest_filename)
if args.update: bad = [] @@ -66,17 +75,25 @@ if args.update: results = checker.check_files([f]) if results.total_errors: bad.append(f) + with file(blacklist_filename, "w") as fh: print >>fh, """\ # cpep8.blacklist: The list of files that do not meet PEP8 standards. # DO NOT ADD NEW FILES!! Instead, fix the code to be compliant. # Over time, this list should shrink and (eventually) be eliminated.""" print >>fh, "\n".join(sorted(bad)) + + if args.scan: + with file(manifest_filename, "w") as fh: + print >>fh, "\n".join(sorted(manifest)) sys.exit(0)
if args.files: manifest = args.files
+# read the blacklisted source files +blacklist = file_to_lines(blacklist_filename) + check_list = [] for f in manifest: if args.all or f not in blacklist:
# HG changeset patch # User Zach Welch zach@mandolincreekfarm.com # Fake Node ID 006e7efdcbbf0438328561d099b1bc44d5b0f478
Add cpep8.sh wrapper script (#2355)
This patch moves the virtualenv setup out of run_all_tests.sh, allowing cpep8.py to be run on its own with the correct module versions. This patch also moves the virtualenv temporary directory into tools/ and adds it to the .hgignore file.
diff --git a/.hgignore b/.hgignore index 4ba935a..140091a 100644 --- a/.hgignore +++ b/.hgignore @@ -4,4 +4,5 @@ .*.rej$ dist build/bdist -tests/logs/ \ No newline at end of file +tools/cpep8.venv/ +tests/logs/ diff --git a/run_all_tests.sh b/run_all_tests.sh index 34a0e42..3a5c67b 100755 --- a/run_all_tests.sh +++ b/run_all_tests.sh @@ -1,7 +1,5 @@ #!/usr/bin/env bash
-VENV="${TMPDIR:-/tmp}/venv" - function record_failure() { FAILED="$1 $FAILED" } @@ -28,29 +26,9 @@ function style_tests() { ./tools/checkpatch.sh }
-function ensure_test_venv() { - virtualenv=$(which virtualenv) - if [ ! -x "$virtualenv" ]; then - echo 'Please install virtualenv' - return 1 - fi - if [ ! -d "$VENV" ]; then - virtualenv "$VENV" - fi - return 0 -} - function pep8() { - ensure_test_venv - if [ $? -ne 0 ]; then - record_failure pep8 - return - fi - source ${VENV}/bin/activate - pip install pep8==1.4.6 >${VENV}/pep8.log 2>&1 echo "Checking for PEP8 regressions..." - time ./tools/cpep8.py - deactivate + time ./tools/cpep8.sh }
if test -z "${TESTS[*]}"; then diff --git a/tools/cpep8.sh b/tools/cpep8.sh new file mode 100755 index 0000000..8522692 --- /dev/null +++ b/tools/cpep8.sh @@ -0,0 +1,21 @@ +#!/usr/bin/env bash +# Runs cpep.py with the proper verion of the pep8 library. + +PEP8_VERSION="1.4.6" + +TOOLS_DIR="$(dirname $0)" +VENV="${TMPDIR:-${TOOLS_DIR}}/cpep8.venv" + +virtualenv="$(which virtualenv)" +if [ ! -x "$virtualenv" ]; then + echo 'Please install virtualenv' + exit 1 +fi +if [ ! -d "$VENV" ]; then + virtualenv "$VENV" +fi + +source ${VENV}/bin/activate +pip install pep8==${PEP8_VERSION} >${VENV}/pep8.log 2>&1 +./tools/cpep8.py "$@" +deactivate
# HG changeset patch # User Zach Welch zach@mandolincreekfarm.com # Fake Node ID 0382c2201867cc52a6178e85202fc112acb45d69
Update cpep8 to use pep8 1.6.2 (#2355)
This patch updates the virtualenv version of pep8 to 1.6.2, chasing down the new style errors that pop up in the already cleaned files.
diff --git a/chirp/generic_xml.py b/chirp/generic_xml.py index b8aec0a..8f96278 100644 --- a/chirp/generic_xml.py +++ b/chirp/generic_xml.py @@ -89,7 +89,7 @@ class XMLRadio(chirp_common.FileBackedRadio, chirp_common.IcomDstarSupport): def get_features(self): rf = chirp_common.RadioFeatures() rf.has_bank = False - #rf.has_bank_index = True + # rf.has_bank_index = True rf.requires_call_lists = False rf.has_implicit_calls = False rf.memory_bounds = (0, 1000) diff --git a/chirp/platform.py b/chirp/platform.py index 5429ebc..d09240b 100644 --- a/chirp/platform.py +++ b/chirp/platform.py @@ -57,8 +57,12 @@ def _find_me():
def natural_sorted(l): - convert = lambda text: int(text) if text.isdigit() else text.lower() - natural_key = lambda key: [convert(c) for c in re.split('([0-9]+)', key)] + def convert(text): + int(text) if text.isdigit() else text.lower() + + def natural_key(key): + [convert(c) for c in re.split('([0-9]+)', key)] + return sorted(l, key=natural_key)
@@ -451,10 +455,10 @@ def _do_test(): print "Log file (foo): %s" % __pform.log_file("foo") print "Serial ports: %s" % __pform.list_serial_ports() print "OS Version: %s" % __pform.os_version_string() - #__pform.open_text_file("d-rats.py") + # __pform.open_text_file("d-rats.py")
- #print "Open file: %s" % __pform.gui_open_file() - #print "Save file: %s" % __pform.gui_save_file(default_name="Foo.txt") + # print "Open file: %s" % __pform.gui_open_file() + # print "Save file: %s" % __pform.gui_save_file(default_name="Foo.txt") print "Open folder: %s" % __pform.gui_select_dir("/tmp")
if __name__ == "__main__": diff --git a/chirp/template.py b/chirp/template.py index 1477629..381b103 100644 --- a/chirp/template.py +++ b/chirp/template.py @@ -109,8 +109,8 @@ class TemplateRadio(chirp_common.CloneModeRadio): mem = chirp_common.Memory()
mem.number = number # Set the memory number - mem.freq = int(_mem.freq) # Convert your low-level frequency - # to Hertz + # Convert your low-level frequency to Hertz + mem.freq = int(_mem.freq) mem.name = str(_mem.name).rstrip() # Set the alpha tag
# We'll consider any blank (i.e. 0MHz frequency) to be empty @@ -125,6 +125,6 @@ class TemplateRadio(chirp_common.CloneModeRadio): # Get a low-level memory object mapped to the image _mem = self._memobj.memory[mem.number]
- _mem.freq = mem.freq # Convert to low-level frequency - # representation + # Convert to low-level frequency representation + _mem.freq = mem.freq _mem.name = mem.name.ljust(8)[:8] # Store the alpha tag diff --git a/chirp/xml_ll.py b/chirp/xml_ll.py index 8713507..e3bac20 100644 --- a/chirp/xml_ll.py +++ b/chirp/xml_ll.py @@ -88,13 +88,13 @@ def get_memory(doc, number): else: mem.skip = skip
- #FIXME: bank support in .chirp files needs to be re-written - #bank_id = _get("/bank/@bankId") - #if bank_id: - # mem.bank = int(bank_id) - # bank_index = _get("/bank/@bankIndex") - # if bank_index: - # mem.bank_index = int(bank_index) + # FIXME: bank support in .chirp files needs to be re-written + # bank_id = _get("/bank/@bankId") + # if bank_id: + # mem.bank = int(bank_id) + # bank_index = _get("/bank/@bankIndex") + # if bank_index: + # mem.bank_index = int(bank_index)
return mem
@@ -175,12 +175,12 @@ def set_memory(doc, mem): skip = memnode.newChild(None, "skip", None) skip.addContent(mem.skip)
- #FIXME: .chirp bank support needs to be redone - #if mem.bank is not None: - # bank = memnode.newChild(None, "bank", None) - # bank.newProp("bankId", str(int(mem.bank))) - # if mem.bank_index >= 0: - # bank.newProp("bankIndex", str(int(mem.bank_index))) + # FIXME: .chirp bank support needs to be redone + # if mem.bank is not None: + # bank = memnode.newChild(None, "bank", None) + # bank.newProp("bankId", str(int(mem.bank))) + # if mem.bank_index >= 0: + # bank.newProp("bankIndex", str(int(mem.bank_index)))
if isinstance(mem, chirp_common.DVMemory): dv = memnode.newChild(None, "dv", None) diff --git a/chirpc b/chirpc index 4981374..217ca9f 100755 --- a/chirpc +++ b/chirpc @@ -40,9 +40,6 @@ def fail_missing_mmap():
class ToneAction(argparse.Action): -# def __init__(self, **kwargs): -# super(ToneAction, self).__init__(**kwargs) - def __call__(self, parser, namespace, value, option_string=None): if value in chirp_common.TONES: raise argparse.ArgumentError("Invalid tone valeu: %.1f" % value) @@ -296,12 +293,12 @@ if __name__ == "__main__": print mem
if options.download_mmap: - #isinstance(radio, chirp_common.IcomMmapRadio) or fail_unsupported() + # isinstance(radio, chirp_common.IcomMmapRadio) or fail_unsupported() radio.sync_in() radio.save_mmap(options.mmap)
if options.upload_mmap: - #isinstance(radio, chirp_common.IcomMmapRadio) or fail_unsupported() + # isinstance(radio, chirp_common.IcomMmapRadio) or fail_unsupported() radio.load_mmap(options.mmap) if radio.sync_out(): print "Clone successful" diff --git a/chirpui/clone.py b/chirpui/clone.py index e9443b0..63f6f10 100644 --- a/chirpui/clone.py +++ b/chirpui/clone.py @@ -60,7 +60,7 @@ class CloneSettingsDialog(gtk.Dialog): port = ports[0] else: port = "" - if not port in ports: + if port not in ports: ports.insert(0, port)
return miscwidgets.make_choice(ports, True, port) diff --git a/chirpw b/chirpw index 073ef50..353edc9 100755 --- a/chirpw +++ b/chirpw @@ -20,10 +20,9 @@ import os from chirp import logger from chirp import elib_intl from chirp import platform +from chirp import * from chirpui import config
-# Hack to setup environment -platform.get_platform()
import sys import os @@ -31,6 +30,7 @@ import locale import gettext import argparse
+ execpath = platform.get_platform().executable_path() localepath = os.path.abspath(os.path.join(execpath, "locale")) if not os.path.exists(localepath): @@ -77,8 +77,6 @@ gettext.textdomain("CHIRP") lang = gettext.translation("CHIRP", localepath, languages=langs, fallback=True)
-import gtk -
# Python <2.6 does not have str.format(), which chirp uses to make translation # strings nicer. So, instead of installing the gettext standard "_()" function, @@ -111,9 +109,6 @@ else: # Python >=2.6, use normal gettext behavior lang.install()
-from chirp import * -from chirpui import mainapp, config - parser = argparse.ArgumentParser() parser.add_argument("files", metavar="file", nargs='*', help="File to open") logger.add_version_argument(parser) @@ -124,7 +119,10 @@ args = parser.parse_args()
logger.handle_options(args)
-a = mainapp.ChirpMain() +a = None +if True: + from chirpui import mainapp + a = mainapp.ChirpMain()
for i in args.files: print "Opening %s" % i @@ -135,10 +133,12 @@ a.show() if args.profile: import cProfile import pstats + import gtk cProfile.run("gtk.main()", "chirpw.stats") p = pstats.Stats("chirpw.stats") p.sort_stats("cumulative").print_stats(10) else: + import gtk gtk.main()
if config._CONFIG: diff --git a/setup.py b/setup.py index b923670..037121f 100644 --- a/setup.py +++ b/setup.py @@ -84,7 +84,7 @@ def macos_build():
EXEC = 'bash ./build/macos/make_pango.sh ' + \ '/opt/local dist/chirp-%s.app' % CHIRP_VERSION - #print "exec string: %s" % EXEC + # print "exec string: %s" % EXEC os.system(EXEC)
@@ -95,7 +95,7 @@ def default_build(): os.system("make -C locale clean all")
desktop_files = glob("share/*.desktop") - #form_files = glob("forms/*.x?l") + # form_files = glob("forms/*.x?l") image_files = glob("images/*") _locale_files = glob("locale/*/LC_MESSAGES/CHIRP.mo") stock_configs = glob("stock_configs/*") diff --git a/tools/cpep8.sh b/tools/cpep8.sh index 8522692..88a7af9 100755 --- a/tools/cpep8.sh +++ b/tools/cpep8.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # Runs cpep.py with the proper verion of the pep8 library.
-PEP8_VERSION="1.4.6" +PEP8_VERSION="1.6.2"
TOOLS_DIR="$(dirname $0)" VENV="${TMPDIR:-${TOOLS_DIR}}/cpep8.venv"
participants (1)
-
Zach Welch