Fix flakes

[jayvdb]

No description provided.

[jayvdb]

If you're not happy with any of this, I am happy to look for alternatives, ignore certain flake8 codes, or even whitelist certain lines using my own beast https://pypi.python.org/pypi/flake8-putty .

[djc]

Hey, thanks for taking a look. I think pyflakes is really useful in finding issues with the code, but with pep8 stuff I generally feel that I keep my code clean enough and that the automatic stuff isn't really a win. So I would prefer not to add any automatic PEP 8 checking, and am not that interested in most of the changes here. Some things I'd be happy to accept (ideally in separate commits):

• Use print() as a function, but preferably with from __future__ import print_function.
• Making function names in the parser unique.
• The move of the node type defines, though I'd want the globals assignment right after the definition of NODE_TYPES, and in the import I want as many names as possible on a line.

I'd be happy if you want to pick those up, but otherwise I can do it myself. :)

[jayvdb]

No worries; I'll adjust it. Thx for the feedback.

[jayvdb]

the chunk above could be avoided by appending flake8's # noqa to this line.

[djc]

That would be much better, thanks!

[jayvdb]

I've ignored the errors pep8 was raising.

With regards to "[moving] globals assignment right after the definition", I've moved it, but I do not agree. Placing them near the top means latter code in the module (added in the future) could overwrite them. The assert helps catch that, but the asserts are only effective if there is no other code after it. An alternative is to add a unit test to assert that the defines haven't been altered during module loading.

Before splitting it off into separate commits, I'd like to reply to your feedback re pep8, hoping to convince you to allow flake8 & pep8 checking, rather than just pyflakes checking.

As you can see from the modified diff, the code is already mostly pep8 compliant, with only minor violations. I agree you are keeping your code clean. The problem is that other people may not be as clean as yourself, especially if you do not signal your code quality expectations before they submit a PR. By including flake8 in .travis.yml or tox.ini, potential submitters have a good idea about how to prepare their patch.

flake8 adds # noqa support, which is a very handy way to bypass pyflakes bugs. I avoids adding dead code. I am a pyflakes developer, and some of the pyflakes bugs you may encounter in the future are not going to be easily fixed in pyflakes; workarounds are needed.

[jayvdb]

Also flake8 has a number of extensions which can help catch silly mistakes, or very poor style. If you really don't want pep8 running, we can disable all pep8 rules by ignoring 'E,W'

[djc]

Would you mind changing this to just id_kw_include, instead of _inc? I generally don't parse inc as an abbrevation of include, but more like increment.

[djc]

Hey, good job arguing for this! The current change is looking much better already...

I kind of like the idea of a unittest to test that the node type constant values are correct. The reason I don't really want it at the bottom is that it seems like one way of having spooky action at a distance. Also, they way they're being used (with string literals used in the parser functions and the constants only being used in the generator), I think with the high coverage, all problems with this will already lead to test failures.

[djc]

Awesome, thanks for all the work!