On Tue, 16 Jul 2013 10:13:50 +0200 Milo Casagrande milo.casagrande@linaro.org wrote:
I usually use pep8 and pyflakes directly from the command line.
What's the general feeling about adding comments which override certain (non-PEP8) checks? PyCharm can inspect code and provide overrides for certain functions, e.g.:
class LavaTestData(object): # noinspection PyDictCreation def __init__(self, test_id='lava'): self.job_status = 'pass' self.metadata = {} self._test_run = {'test_results': [], 'attachments': [], 'tags': []} self._test_run['test_id'] = test_id self._assign_date() self._assign_uuid()
This one is relatively trivial but there are some where the relevant change would be more intrusive.
# Set process id if job-id was passed to dispatcher if self.args.job_id: try: # noinspection PyUnresolvedReferences from setproctitle import getproctitle, setproctitle except ImportError: logging.warning( ("Unable to set import 'setproctitle', " "process name cannot be changed")) else: setproctitle("%s [job: %s]" % ( getproctitle(), self.args.job_id))
The advantage is that the IDE then makes it clearer if PEP8 problems are introduced, the disadvantage is IDE-specific comments. (I'm not sure if other tools will make use of such comments.) The comments can be modified in-place to show what is being overridden in the IDE.
As Zygmut said, pflake8 is a handy tool (if I'm not wrong it runs pep8 I personally prefer to stick with distro provided tools, without having to go through pip or easy_install, but I guess this will become a problem since we are all using different distros with their own package versions.
I'm using flake8 2.0 - it adds a useful feature to skip certain files entirely (e.g. deprecated code like lava_test) which otherwise trips up pep8. (Just add # flake8: noqa as a line near the top of the script.)
One other element which I have so far ignored is this behaviour:
try: data = self.start_testcase(test_case_id) except: logging.exception("start_testcase failed for %s", test_case_id)
This raises a warning (in PyCharm and pylint) about "too broad exception clause" / "No exception type(s) specified".
Maybe we should document which pylint outputs we're happy to ignore.
I'm more keen on having strict PEP8 checking so that the code will (almost) have the same style, and will be a little bit easier to read. I would stick to the 79 chars line length, plus all the "default" PEP8 settings (multiple spaces, no space between operators...).
I'm less keen on the 79 character limit - it's going to need quite a few new sub-routines to reduce the indentation.
I'm using this syntax currently:
~/.local/bin/flake8 -v --ignore E501 --show-source
Packaged: pep8 1.4.5-1 pyflakes 0.7.2-1
I've now got lava-dispatcher to a point where there are no problems reported by pep8 or flake8 (modulo the line length) *and* PyCharm is also happy to show all the files as without warnings.
I'll push the changes selectively, there is a proposed merge for files which only need whitespace changes, I'll push files which also need source code changes next (after testing in the multinode branch).