Work towards PEP8

[mgoffin]

I tried to catch as many things as I could while my brain was frying over the code I was going through. Hopefully I didn't miss any glaring problems! The majority of changes are:

• Using {} and .format() for strings instead of %s and %.
• Using print() instead of print
• Removing unnecessary whitespace, but also adding necessary whitespace
• Cleaner indentation to try to keep lines below 80 characters and improve readability/maintenance.
• Removal of imported libraries that weren't used.
• Commenting out variables that were assigned but never used.
• Fixing variables that were fat-fingered.
• Marking #TODO in locations where there are undefined name errors as I'm not sure where they are supposed to come from or if pyflakes is just going nuts (lots of them seem to be globals but also not defined in the file).
• Moved some code around to keep classes and functions together in the same file.

I suspect this will need some going through as it is a large set of changes, but I tried not to modify any code that would cause functionality changes.

[marnao]

Thanks for taking this on @mgoffin

I'll need to look through this more carefully but one thing I noticed right away is that you're going to break python 2.6 compatibility with your string formatting. It does not support zero length field names (e.g. you'd need {0} instead of {})

[mgoffin]

Ahh, I didn't realize you were still supporting 2.6. That's my mistake! I can go back and fix this.

[marnao]

@mgoffin did you use an automated tool to make these changes or was it manual? Or a combination of automated and manual?

Trying to figure out how much scrutiny I need to give this PR.

[mgoffin]

This was all manual. I didn't want to run anything automated that would make changes I didn't understand.

[marnao]

In general I don't have a problem with working towards PEP 8 (aside from the 79 character line.. but that's bordering on political debate).

However, I am concerned about the sweeping changes across so many files and my ability to test all of them sufficiently in a timely manner. My eyeball didn't catch this one for instance, but it broke laikad nonetheless:

$ python laikad.py Traceback (most recent call last): File "laikad.py", line 1113, in main() File "laikad.py", line 1053, in main gracetimeout) File "laikad.py", line 472, in init (randint(0, 0x10000), randint(0, 0x10000))) ValueError: Unknown format code 'X' for object of type 'str'

Hard breaks like this one are great because we can go fix it.. I'm worried about the more subtle ones that change how the program operates but do not break it completely.

Have you done this sort of thing before on such a large scale? How did it work out?

[mgoffin]

Hmm, I remember testing that line of code. I'll give it another look.

I had to do this for the pytx library, though it was code I was very familiar with having written it all, and the amount of prints in that code is minimal. It required a little testing to make sure any edge cases were caught, but that's the nature of the game with projects like this :)

I'll see what I find with this error and get a fix pushed to the branch!

[mgoffin]

This is pretty weird. So the format docs (https://docs.python.org/2/library/string.html#formatspec) have an example identical to what we are doing:

>>> octets = [192, 168, 0, 1]
>>> '{:02X}{:02X}{:02X}{:02X}'.format(*octets)
'C0A80001'
>>>

And if I try this in python shell it works fine:

>>> foo = "{0:04X}-{1:04X}".format(randint(0, 0x10000), randint(0, 0x10000))
>>> foo
'3FDA-9283'
>>> foo = "{0:04X}-{1:04X}".format(randint(0, 0x10000), randint(0, 0x10000))
>>> foo
'C004-EBD5'
>>> foo = "{0:04X}-{1:04X}".format(randint(0, 0x10000), randint(0, 0x10000))
>>> foo
'F250-3D90'
>>>

Trying to determine what laikad.py is doing that is causing this to not work.

[marnao]

In your quest for 79 character lines you added an extra set of parentheses ;-)

[mgoffin]

Well played!

[mgoffin]

Is there anything else we can look at here to get this merged? :)

[marnao]

What do you think about breaking this up into more manageable chunks? I think that would make reviewing and testing a whole lot easier. Maybe a couple modules at a time to start? A partner of ours developed a module testing framework that we've been using with some success. Hoping they open source it soon.

The framework and the executable scripts worry me the most because they're complex and we don't have good tests for them.

[mgoffin]

It's going to be a giant pain to push the changes aside and submit PRs one or two files at a time.