[issue20815] ipaddress unit tests PEP8
Roundup Robot added the comment: 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 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
R. David Murray added the comment: 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. -- nosy: +r.david.murray resolution: - fixed stage: commit review - resolved status: open - closed versions: +Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Raymond Hettinger added the comment: 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. -- priority: normal - low ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Changes by Ezio Melotti ezio.melo...@gmail.com: -- stage: - commit review type: - enhancement ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Michel Albert added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
pmoody added the comment: Will Do. Michael, would you mind signing the contributor agreement so I can apply your patch? http://www.python.org/psf/contrib/contrib-form/ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Michel Albert added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
pmoody added the comment: thanks. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
pmoody added the comment: Nick, did you want me to apply this? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Nick Coghlan added the comment: Yeah, I like the small readability tweaks in the r3 patch, so feel free to apply that one. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Michel Albert added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Michel Albert added the comment: Here's a new patch which addresses white-space issues without touching the old tests. -- Added file: http://bugs.python.org/file34265/test_ipaddress_pep8-r3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Raymond Hettinger added the comment: 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. -- nosy: +rhettinger ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Nick Coghlan added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
New submission from Michel Albert: 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. -- components: Tests files: test_ipaddress_pep8.patch keywords: patch messages: 212497 nosy: exhuma, ncoghlan, pmoody priority: normal severity: normal status: open title: ipaddress unit tests PEP8 versions: Python 3.5 Added file: http://bugs.python.org/file34257/test_ipaddress_pep8.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Nick Coghlan added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Michel Albert added the comment: 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? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20815] ipaddress unit tests PEP8
Nick Coghlan added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20815 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
