While I was looking at the source of the ipaddress unit-tests, I noticed a couple of PEP8 violations. This patch fixes these (verified using the ``pep8`` tool).

There are no behavioural changes. Only white-space.

Test-cases ran successfully before, and after the change was made.

Unfortunately, the pep8 tool includes some additional invented rules of its own, mostly related to being far too strict about indentation of continuation lines.

PEP 8 is actually mostly silent on that topic, merely pointing out a couple of specific things *not* to do. It's the tool that extrapolates a whole host of continuation line indentation rules that even PEP 8 itself doesn't meet.

There's also no blanket requirement in the PEP to have spaces around all binary operators (since that doesn't always aid readability), just a limited subset of them.

I've recently filed a bug against the tool itself, suggesting that given its name, its default behaviour should be limited to things that are actually backed up by the PEP.

That said, I think some of the cases picked up here do represent genuine readability improvements due to improved alignment. They just need to be separated out from the spurious changes resulting from invented rules in the pep8 tool.

Thanks for the quick reply!

I did not know the pep8 tool added it's own rules :( I have read PEP8 a long while ago and have since relied on the tool to do "the right thing". Many of it's idiosyncrasies have probably made their way into my blood since :(

And you're right: The PEP is actually explicitly saying that it's okay to leave out the white-space to make operator precedence more visible (for reference: http://legacy.python.org/dev/peps/pep-0008/#other-recommendations).

I will undo those changes.

Is there anything else that immediately caught your eye so I can address it in the update?

I'd suggest focusing more on the structured tests at the top of the file - the ones further down we largely incorporated wholesale from the original ipaddr project, and didn't go back and enforce PEP 8 compliance, but the new tests should follow the style guide.

Here's a new patch which addresses white-space issues without touching the old tests.

FWIW, I consider this patch to be a waste of time and don't want to encourage more patches like it.   Please focus on substantive changes like improving test coverage, adding documentation strings, etc.

Applying this patch would make a microscopic improvement to neatness but it also has costs (wasting Nick's time, masking the actual test developer's name in the Hg file annotation, and making it more difficult to apply cross-version bug fixes).

PEP-8 is a set of guidelines, not a rigid set of rules where "violations" must be sought out and eliminated.   That is not the spirit of the PEP.

I recommend closing this as won't fix.

I think there's still value in giving someone experience with the current patch development process, as well as looking for potentially genuine readability improvements.

However, I also agree that automated compliance with a style guide checker can't sensibly be retrofitted to an existing project, even if that project is the originator of the style guide (it generates too much noise in the code history, and risks creating spurious merge conflicts for other changes. In the specific case of the standard library, we also sometimes make exceptions to the style guide to reduce code churn when merging in an existing project, as happened in the case of ipaddress).

In this case, I like the alignment fixes in the custom assertion methods, so I'll probably apply those as a small readability improvement.

I strongly agree with Raymond's points! They are all valid.

I should note, that I submitted this patch to - as mentioned by Nick - familiarise myself with the patch submission process. I decided to make harmless changes which won't risk braking anything.

Nick, did you want me to apply this?

Yeah, I like the small readability tweaks in the r3 patch, so feel free to apply that one.

Will Do.

Michael, would you mind signing the contributor agreement so I can apply your patch?

http://www.python.org/psf/contrib/contrib-form/

Did so already last weekend. I suppose it will take some time to be processed.

I can ping you via a message here once I receive the confirmation.

thanks.

It seems the contributor agreement form has been processed. As I understand it, the asterisk on my name confirms this.

I also verified that this patch cleanly applies to the most recent revision.

Marking as low priority.  This patch really isn't worth applying.

If someone wants to starting contributing, there are plenty of useful things to do (such as adding docstrings to elementtree).  I don't want to encourage non-substantive shallow code churn and eats developer time for no benefit.

From my point of view, the "benefit" is actually negative because these sort of micro-rearrangements clutter the "annotate" history and because it makes it more difficult to apply substantive patches across multiple versions of Python.

New changeset 27a02eb9273f by R David Murray in branch '3.4':
#20815: small readability improvements in ipaddress tests.
https://hg.python.org/cpython/rev/27a02eb9273f

New changeset 4c9d27eef892 by R David Murray in branch 'default':
#20815: small readability improvements in ipaddress tests.
https://hg.python.org/cpython/rev/4c9d27eef892

Since ipaddress is relatively new, the package maintainers approved the patch, and it is mostly blank lines (and therefore not a problem with
annotate) I committed this.  Thank Michel.