Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Haigha doesn't abide by PEP8 #49

Closed
tmehlinger opened this issue Mar 31, 2014 · 14 comments
Closed

Haigha doesn't abide by PEP8 #49

tmehlinger opened this issue Mar 31, 2014 · 14 comments

Comments

@tmehlinger
Copy link

Understanding and accepting that there are variations of style across projects, it would be nice to see Haigha updated to respect the guidelines set forth in PEP8. It is very difficult to work on Haigha without altering linter configuration to ignore basically all the warnings and errors.

http://legacy.python.org/dev/peps/pep-0008/

@awestendorf
Copy link
Member

I understand but I fundamentally disagree with the amount of whitespace required by pep8.

@CrackerJackMack
Copy link

I guess the real question is -- Should a personal opinion about a community adopted standard take precedence for something that is made public?

I like Haigha, it's well written and easy to follow. Just very difficult to hack on or contribute to in it's current state.

@awestendorf
Copy link
Member

Do you have an example of where that difficulty shows up? My read of the PEP8 intent is that project-specific conventions and readability take precedence over the conventions dictated for the standard library. I would love to see more community contributions so if there's a major blocking issue, that's a big deal to me.

@CrackerJackMack
Copy link

https://gist.github.com/CrackerJackMack/9901819

find devel/haigha/ -type f -name '*.py' | grep -v tests | xargs flake8 --max-complexity=12 --statistics

796     E111 indentation is not a multiple of four
72      E121 continuation line indentation is not a multiple of four
6       E125 continuation line does not distinguish itself from next logical line
59      E127 continuation line over-indented for visual indent
22      E128 continuation line under-indented for visual indent
330     E201 whitespace after '['
342     E202 whitespace before ']'
87      E203 whitespace before ':'
1       E221 multiple spaces before operator
32      E225 missing whitespace around operator
16      E227 missing whitespace around bitwise or shift operator
25      E228 missing whitespace around modulo operator
72      E231 missing whitespace after ':'
4       E251 unexpected spaces around keyword / parameter equals
1       E261 at least two spaces before inline comment
11      E301 expected 1 blank line, found 0
32      E302 expected 2 blank lines, found 1
7       E303 too many blank lines (2)
3       E401 multiple imports on one line
100     E501 line too long (82 > 79 characters)
5       E502 the backslash is redundant between brackets
46      E701 multiple statements on one line (colon)
4       E711 comparison to None should be 'if cond is None:'
30      F401 'time' imported but unused
6       F403 'from haigha.frames import *' used; unable to detect undefined names
2       F841 local variable 'e' is assigned to but never used
3       W291 trailing whitespace
21      W293 blank line contains whitespace
4       W391 blank line at end of file

Using the right tooling (flake8 for example), everyone can work on a project without having to deal with style issues and instead logic and features.

If you are willing to accept a pull request for this I'll happily attempt it, but it will make the git history seem like I stole the project.

@sudorandom
Copy link

Here's a more visual representation of what a typical Python programmer sees:

If you want other Python programmers to contribute it seems like you're intentionally making it difficult for them by going against the grain. Effective Python programmers have this kind of tooling built into their editor and CI not only to potentially catch mistakes via the static analysis of flake8/pylint but also to stay in line with the community accepted style.

@dehun
Copy link

dehun commented Apr 15, 2014

I just fixed all pep8 errors except E501(line too long)
please see pull request here:
#51

@awestendorf
Copy link
Member

Thank you. I wasn't ignoring this thread, just that work, family and heartbleed have gotten in the way.

@awestendorf awestendorf mentioned this issue May 3, 2014
Merged
@awestendorf
Copy link
Member

A little late, but master is now pep8'ed.

@CrackerJackMack
Copy link

Smashing job sir

@josegonzalez
Copy link
Contributor

This commit breaks BC.

In previous versions, you could do:

from haigha.connections import RabbitConnection

Now this throws a lovely ImportError. Granted it's probably not the smarted thing to do the above, but we did indeed do this.

Here is what code should do:

from haigha.connections.rabbit_connection import RabbitConnection

Real-world bug with PR: seatgeek/amqp-dispatcher#13

Good effort though, I 👍 any move to PEP8 :)

@awestendorf
Copy link
Member

Yeah, pep8 (or at least flake8) is very particular about importing into a module __init__ when the import goes unused. I strongly disagree with that, as I think there is a lot of value in hiding the full path of an implementation and loading only the necessary components into a more friendly namespace.

@josegonzalez
Copy link
Contributor

You could use this trick: https://github.com/josegonzalez/beaver/blob/master/setup.py#L35

@vitaly-krugl
Copy link

Pylint has pragmas for disabling warnings on case-by-case. Can you do the same with flake8?

From: Aaron Westendorf <notifications@github.commailto:notifications@github.com>
Reply-To: agoragames/haigha <reply@reply.github.commailto:reply@reply.github.com>
Date: Wednesday, June 4, 2014 at 12:26 PM
To: agoragames/haigha <haigha@noreply.github.commailto:haigha@noreply.github.com>
Subject: Re: [haigha] Haigha doesn't abide by PEP8 (#49)

Yeah, pep8 (or at least flake8) is very particular about importing into a module init when the import goes unused. I strongly disagree with that, as I think there is a lot of value in hiding the full path of an implementation and loading only the necessary components into a more friendly namespace.


Reply to this email directly or view it on GitHubhttps://github.com//issues/49#issuecomment-45139432.

@awestendorf
Copy link
Member

I'm loathe to have any special rules just for haigha; I figure if there's a style guide that the community feels strongly about, then both the pros and the cons resulting from that spec must be acceptable to the community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants