[sugar] Python Style Guide

[Dan Williams]

On Tue, 2006-11-14 at 12:11 +0100, Marco Pesenti Gritti wrote:
> Ian Bicking wrote:
> > Ivan mentioned that there was someone willing to update all the OLPC 
> > code once we have a style guide.
> >
> > With that in mind I put together a draft of a style guide: 
> > http://wiki.laptop.org/go/Python_Style_Guide
> >
> > This is primarily a copy of PEP 8: 
> > http://www.python.org/dev/peps/pep-0008/ -- I think we discussed most 
> > of the variances the current code has from PEP 8 a couple months ago, 
> > and I think everyone was okay with changing the code in those ways.  
> > But of course, there were other things to attend to at the time; 
> > having someone else volunteer to do the change makes this relevant again.
> >
> > I've marked a number of things I'm unsure of with [note: ...].  
> > Several of these are about code (and other file) layout issues, which 
> > don't matter too much to an initial changeover of the code. 
> > Internationalization style remains; I think someone besides myself 
> > would be better at specifying the style for these.
> >
> > We might want to consider setting up a pylint profile for this style. 
> > pylint (http://www.logilab.org/projects/pylint) is notable among 
> > Python source checkers for being anal about style and other small 
> > details. Whoever is updating the code for the style might start out by 
> > tweaking pylint until it can detect all the details we want to 
> > update.  It's also better to stick to the easy/mechanical updates to 
> > start with, and avoid updates that might introduce bugs.  Probably it 
> > would make sense to annotate the style guide rules with "code already 
> > should conform", "we will update current code" and "code will be 
> > updated when possible".
> >
> 
> Cool! Some comments:
> 
>  > Use 4 spaces per indentation level. Do not use tabs.
> 
> Heh, I'm fine with this... we just need to convince Dan.

Whatever, don't let me be a blocker for this.

>  > Separate top-level function and class definitions with two blank lines.
> 
> Is this taken from PEP-8?
> 
>  >__init__.py files should generally contain no substantive code. 
> Instead they should import from >other modules. Importing from other 
> modules is done so that a package can provide a front-facing >set of 
> objects and functions it exports, without exposing each of the internal 
> modules in the >package. Note however that this causes the submodules to 
> be eagerly imported; if this is likely to >cause unnecessary overhead 
> then the import in __init__.py should be reconsidered.
> 
> Can you give an example of this? I think it would be useful to have it 
> on the guide too. Is it part of PEP-8 in any way?
> 
>  > Version bookkeeping
> 
> Doesn't sound particularly interesting to me. I'd probably just omit  
> that section.
> 
>  > Module names
> 
> Might be worth motivating lowercase with other reasons than filesystem 
> compatibility (I think you gave better reasons on the list a while ago).
> 
>  > [note: I don't think double underscore should ever be used; maybe 
> this should be removed]
> 
> I think it should, it's just confusing.

I am a bit concerned about visibility though; for stuff that should
_never_ be used from programs outside of the sugar bindings we should
really be using __ to keep the symbols private.  There _are_ things that
we need to do that we really don't want to be part of the public API,
and that we want to enforce; especially where the implementation details
might change quite rapidly over the next few months.  But in general, __
should go away.

>  >For simple public data attributes, it is best to expose just the 
> attribute name, without complicated >accessor/mutator methods
> 
> I'm not sure I like this... It's quite unusual for non python coders. If 
> we want to keep it we should probably elaborate more on it in the guide.

Again, if we expect any of this stuff to change we should be hiding them
behind accessors...  I know there are a _ton_ of tricks you can use in
Python to maintain backward compatibility, and I've seen & used a few of
them before in my python projects.  This is a balance between work we'd
have to do in the future to maintain backwards compat and a small amount
of adding accessors here and there.

I'm very uncomfortable with exposing stuff like:

class Foo:
	def __init__(self):
		self.var = 1

a = Foo()
a.var = 2

because then you can't do any validation and you never know when the
variable changes; accessors fix this because you always know when the
value changes and you can validate the change and throw exceptions.  If
this isn't what you're talking about, ignore me.

For example; dbus always returns strings as unicode because the native
bus encoding is UTF-8, and you can't just do 'if a == b' where a is
unicode and b is encoded.  Therefore, for stuff like that, you always
need to use an accessor to either enforce the restriction or to do the
conversion yourself.

> *> Comparisons to singletons like None should always be done with 'is' 
> or 'is not', never the equality operators.*
> 
> Is this just style or does it have other consequences?

"a == None" is not the same thing as "a is None"; I've been tripped up
by this before but can't quite remember what it was...

John P. also pointed out that for type-checking, I need to use
'isinstance' so that type hierarchies evaluate correctly.

> *> For sequences, (strings, lists, tuples), use the fact that empty 
> sequences are false.
> 
> Nice one!
> 
> *> Is _() always defined?
> 
> Nope, I think you need to import it from gettext.
> 
> Anyway, great stuff. Let's try to finalize and start using it.
> 
> Marco
> _______________________________________________
> Sugar mailing list
> Sugar at laptop.org
> http://mailman.laptop.org/mailman/listinfo/sugar