canonical-web-and-design / practices

Guides and principles from the web team at Canonical and Ubuntu
https://canonical-web-and-design.github.io/practices/
Other
44 stars 30 forks source link

Document python coding standard #73

Closed Lukewh closed 4 years ago

Lukewh commented 6 years ago

There appear to be conflicting ideas on some styling.

As a casual pythoner I'd appreciate some form of guidance.

squidsoup commented 6 years ago

Have you guys considered something like Black? Very much in the vein of adopting Prettier for js (:skull: to bikeshedding).

nottrobin commented 6 years ago

Just to let you know, our current policy (which isn't written down in practices yet) is that python code format should pass flake8 in its default settings (which is basically PEP8). For the time being, any format differences that don't matter to flake8 don't matter to us either, and we shouldn't bike shed about them.

But I'm just trying out Black to see if we should move our standard to be based off that instead.

nottrobin commented 6 years ago

I like the sound of Black. One significant place it differs from PEP8 is that it prefers max line length of 88 chars, rather than 79. This would mean we'd have to deviate slightly from the PEP8 standard, meaning that we'd have to do one of:

@WillMoggridge, @tbille thoughts?

nottrobin commented 6 years ago

After discussing with @tbille, I think maybe configuring Black to use 79 chars is better, as that's the standard in Prettier, and for Git commits etc.

WillMoggridge commented 6 years ago

I do have opinions on things beyond flake8 and I think strong consistency is helpful. It is nice as a maintainer and can remove doubts how to do something as a casual committer.

Black actually does all of my preferences, other than line length. Such as trailing commas and closing parenthesis on new lines. I actually enjoy the 79 character limit. Black can be configured to use that line limit.

I'm fond of the line limit because it fits perfectly side by side on a 1080p monitor with a sidebar and minimaps. :)

If we use Black, it makes sense to use Black as the CI linter as well.

WillMoggridge commented 6 years ago

Ah yes, I concur as you can see.

nottrobin commented 6 years ago

@tbille also mentioned that Prettier for Python is on the way: https://github.com/prettier/prettier-python. However, since it says "WORK IN PROGRESS", I think we're probably better going with Black for now unless anyone objects?

nottrobin commented 6 years ago

@nkantar mentioned White to me. It sounds like we should use that instead ;)

squidsoup commented 6 years ago

Uh oh, now the python community has a microlibrary fetish. Thanks javascript.

On Thu, Sep 6, 2018 at 10:32 AM Robin Winslow notifications@github.com wrote:

@nkantar https://github.com/nkantar mentioned https://github.com/ambv/black/issues/498#issuecomment-418892045 White https://github.com/kennethreitz/white#white-black-but-brighter-pep8inspired to me. It sounds like we should use that instead ;)

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/canonical-webteam/practices/issues/73#issuecomment-418902931, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH87hmWLwmxec1utbxEjJtPEx0sRkn-ks5uYFEWgaJpZM4VJzLY .

nkantar commented 6 years ago

@nottrobin FWIW, I don't know that White is still actively maintained (or whether it needs to be). But I was thinking about implementing the same thing myself, for better or worse. 😬

nottrobin commented 6 years ago

@nkantar I think I'm only joking about actually using White. Black has guides for setting it up in most editors etc., it's probably simpler for everyone, and will be less error prone, to use the original project.

nkantar commented 6 years ago

@nottrobin Haha, that's a relief! 👍

b-m-f commented 5 years ago

Added a PR => https://github.com/canonical-webteam/practices/pull/120