PEP8 compliance on colors.py #1166
Conversation
| 'whitesmoke' : '#F5F5F5', | ||
| 'yellow' : '#FFFF00', | ||
| 'yellowgreen' : '#9ACD32', | ||
| 'aliceblue': '#F0F8FF', |
There was a problem hiding this comment.
I don't think we will want to de-dent this dictionary literal, the former made it easier to scan the colour names and/or hex values.
There was a problem hiding this comment.
but the former is not pep8 :(
I'll update the PR
There was a problem hiding this comment.
Hold fire until we get some other feedback then. If anyone else agrees with me then do it, if not, then its just me being picky ;-)
There was a problem hiding this comment.
In fact this is tolerated by pep8 and it is much nicer. i've updated the PR
There was a problem hiding this comment.
On 2012/08/30 3:11 AM, Varoquaux wrote:
but the former is not pep8 :(
But it is. Quoting from
http://www.python.org/dev/peps/pep-0008/#a-foolish-consistency-is-the-hobgoblin-of-little-minds:
But most importantly: know when to be inconsistent -- sometimes the
style guide just doesn't apply. When in doubt, use your best judgment.
Look at other examples and decide what looks best. And don't hesitate to
ask!
Two good reasons to break a particular rule:
When applying the rule would make the code less readable, even for
someone who is used to reading code that follows the rules.
...
Eric
|
Apart from my first comment, +1 |
|
|
||
| class ColorConverter: | ||
| class ColorConverter(object): |
There was a problem hiding this comment.
Inheriting from object, while probably benign, will change behavior for those inheriting from these classes under Python 2.x. This should have an entry in docs/api/api_changes.rst.
|
I've rebased and added the inheritance in the api_changes doc |
This is a new pull request on the pep8 compliance of colors.py, as asked by @pelson