Diff for "PythonStyleGuide"

Not logged in - Log In / Register
Differences between revisions 25 and 36 (spanning 11 versions)
⇤ ← Revision 25 as of 2015-03-09 14:49:04 →
Size: 16238
Editor: cjwatson
Comment: more post-apocalypse stuff
← Revision 36 as of 2021-11-25 16:16:21 → ⇥
Size: 74
Editor: cjwatson
Comment: redirect to readthedocs
Deletions are marked like this. Additions are marked like this.
Line 1: Line 1:
= Python Style Guide =

This document describes expected practices when writing Python code. There are occasions when you can break these rules, but be prepared to justify doing so when your code gets reviewed.

<<TableOfContents>>

== Existing Conventions ==

There are well-established conventions in the Python community, and in general we should follow these. General Python conventions, and required reading:

 * [[http://www.python.org/peps/pep-0008.html|PEP 8]]: Style Guide for Python Code
 * [[http://www.python.org/peps/pep-0257.html|PEP 257]]: Docstring Conventions
 * The Zen of Python: {{{python -c "import this"}}}

Note that our standards differ slightly from PEP-8 in some cases.

Coding standards other projects use:

 * [[http://twistedmatrix.com/trac/browser/trunk/doc/core/development/policy/coding-standard.xhtml?format=raw|Twisted Coding Standard]]
 * [[http://wiki.zope.org/zope3/CodingStyle|Zope 3 Coding Standards]]

Although Launchpad is still written in Python 2, there are several things you can do to improve compatibility with Python 3, making an eventual port easier. There are some good guidelines in the [[https://wiki.ubuntu.com/Python/3|Ubuntu wiki]] about Python 3.

== Related Documents on This Wiki ==

 * ExceptionGuidelines
 * AssertionsInLaunchpad
 * [[LaunchpadHackingFAQ]]
 * TestsStyleGuide

== Whitespace and Wrapping ==

 * Code should fit within 78 columns, so as to fit nicely in an 80 column terminal, even when quoted in an email.
 * Indents should be 4 spaces.
 * '''No tabs'''. This is not negotiable.

<<Anchor(multiline)>>
=== Multiline braces ===

There are lots of cases where you might have a list, tuple or dictionary
literal that spans multiple lines. In these cases, you should consider
formatting these literals as follows. This format makes changes to the list
clearer to read in a diff. Note the trailing comma on the last element.

{{{#!python
    mydict = {
        'first': 1,
        'second': 2,
        'third': 3,
        }

    mylist = [
        'this is the first line',
        'this is the second line',
        'this is the third line',
        ]
}}}


== Naming ==

Consistency with existing code is the top priority. We follow PEP-8 with the following exceptions:


 * {{{CamelCase}}}: classes, interfaces (beginning with {{{I}}})
 * {{{initialCamelCase}}}: methods
 * {{{lowercase_underscores}}}: functions, non-method attributes, properties, local variables
 * {{{ALL_CAPS}}}: constants


=== Private names are private ===

You should never call a non-public attribute or method from another class. In
other words, if class A has a method `_foo()`, don't call it from anywhere
outside class A.

== Docstrings ==

 * If you haven't already, read [[http://www.python.org/peps/pep-0257.html|PEP 257]]
 * In general, everything that can have a docstring should: modules, classes, methods, functions.
 * Docstrings should always be enclosed in triple double quotes: {{{"""Like this."""}}}
 * When a class or a method implements an interface, the docstring should say {{{"""See `IFoo`."""}}}

Docstrings should be valid [[http://docutils.sourceforge.net/rst.html|reST]] (with all the painful indentation rules that implies) so that tools such as [[http://codespeak.net/~mwh/pydoctor/|pydoctor]] can be used to automatically generate API documentation.

You should use field names as defined in the [[http://epydoc.sourceforge.net/fields.html|epydoc]] documentation but with reST syntax.

Using {{{`name`}}} outputs a link to the documentation of the named object, if pydoctor can figure out what it is. For an example of pydoctor's output, see http://starship.python.net/crew/mwh/hacks/example.html.

Here is comprehensive example. Parameter descriptions are a good idea but not mandatory. Describe in as much or as little detail as necessary.

{{{#!python
def example2(a, b):
    """Perform some calculation.

    It is a **very** complicated calculation.

    :param a: The number of gadget you think this
              function should frobnozzle.
    :type a: ``int``
    :param b: The name of the thing.
    :type b: ``str``
    :return: The answer!
    :rtype: ``str``.
    :raise ZeroDivisionError: when ``a`` is 0.
    """
}}}

== Modules ==

Each module should look like this:

{{{#!python
# Copyright 2009-2011 Canonical Ltd. All rights reserved.

"""Module docstring goes here."""

__metaclass__ = type
__all__ = [
    ...
    ]
}}}

The file {{{standard_template.py}}} has most of this already, so save yourself
time by copying that when starting a new module. The "..." should be filled in
with a list of public names in the module.

Note that although PEP-8 says to "put any relevant {{{__all__}}} specification
after the imports", Launchpad code should have the {{{__all__}}} before the
imports. This makes it easy to see what a module contains and exports, and
avoids the problem that differing amounts of imports among files means that
the `__all__` list is in a different place each time. Given that we have the
Import Fascist utility (see [[#imports|Imports]]), we use {{{__all__}}} more
often than in general Python code. {{{__all__}}} should be formatted like imports.

<<Anchor(imports)>>
== Imports ==

=== Restrictions ===

There are restrictions on which imports can happen in Launchpad. Namely:

 * View code cannot import code from {{{lp.*.model}}}.
 * {{{import *}}} cannot be used if the module being imported from does not have an {{{__all__}}}
 * Database code may not import {{{zope.exceptions.NotFoundError}}} -- it must instead use {{{lp.app.errors.NotFoundError}}}

These restrictions are enforced by the Import Fascist, which will cause your tests not to pass if you don't abide by the rules.

Imports should be fully qualified. Good:

{{{#!python
# foo/bar.py
import foo.baz
}}}

Bad:

{{{#!python
# foo/bar.py
import baz
}}}

I.e. if {{{foo.bar}}} imports {{{foo.baz}}}, it should say {{{import foo.baz}}}, '''not''' {{{import baz}}}.

=== Multiline imports ===

Sometimes import lines must span multiple lines, either because the package
path is very long or because there are multiple names inside the module that
you want to import.

'''Never use backslashes in import statements!''' Use parenthesized imports:

{{{#!python
from foo import (
   That,
   TheOther,
   This,
)
}}}

Like other lists, imports should list one item per line. The exception is if only
one symbol is being imported from a given module.

{{{#!python
from lp.app.widgets.itemswidgets import CheckBoxMatrixWidget
}}}

But if you import two or more, then each item needs to be on a line by itself. Note the
trailing comma on the last import and that the closing paren is on a line by itself.
{{{#!python
from lp.app.widgets.itemswidgets import (
    CheckBoxMatrixWidget,
    LaunchpadRadioWidget,
    )
}}}


Also, 'make lint' is a very good tool for helping you maintain your imports.

=== Import scope ===

We encourage importing names from the location they are defined in. This
seems to work better with large complex components.


=== Circular imports ===

With the increased use of native Storm APIs, you may encounter more circular
import situations. For example, a `MailingList` method may need a reference
to the `EmailAddress` class for a query, and vice versa. The classic way to
solve this is to put one of the imports inside a method instead of at module
global scope (a "nested import").

Short of adopting something like Zope's lazy imports (which has issues of its
own), you can't avoid this, so here are some tips to make it less painful.

 * Do the nested import in the least common case. For example, if 5 methods
 in `model/mailinglist.py` need access to `EmailAddress` but only one method
 in `model/emailaddress.py` needs access to `MailingList`, put the import
 inside the `emailaddress.py` method, so you have fewer overall nested
 imports.
 * Clearly comment that the nested import is for avoiding a circular import,
 using the example below.
 * Put the nested import at the top of the method.

{{{#!python
        def doFooWithBar(self, ...):
            # Import this here to avoid circular imports.
            from lp.registry.model.bar import Bar
            # ...
            return store.find((Foo, Bar), ...)
}}}

=== Circular imports and webservice exports ===
One of the largest sources of pain from circular imports is caused when you need to export an interface on the webservice. Generally, the only way around this is to specify generic types (like the plain old `Interface`) at declaration time and then later patch the webservice's data structures at the bottom of the interface file.

Fortunately there are some helper functions to make this less painful, in `lib/lp/services/webservice/apihelpers.py`. These are simple functions where you can some info about your exported class/method/parameters and they do the rest for you.

For example:
{{{#!python
    from lp.services.webservice.apihelpers import (
        patch_entry_return_type, patch_collection_return_type)
    patch_collection_return_type(
        IArchive, 'getComponentsForQueueAdmin', IArchivePermission)
    patch_entry_return_type(
        IArchive, 'newPackageUploader', IArchivePermission)
}}}


== Properties ==

Properties are expected to be cheap operations. It is surprising if a property is not cheap operation. For expensive operations use a method, usually named {{{getFoo()}}}. Using {{{cachedproperty}}} provides a work-around but it should not be overused.


== Truth conditionals ==

Remember that False, None, [], and 0 are not the same although they all evaluate to False in a boolean context. If this matters in your code, be sure to check explicitly for either of them.

Also, checking the length may be an expensive operation. Casting to bool may avoid this if the object specializes by implementing __nonzero__.


== Chaining method calls ==

Since in some cases (e.g. class methods and other objects that rely on descriptor __get__() behaviour) it's not possible to use the old style of chaining method calls (SuperClass.method(self, ...)), we should always use the super() builtin when we want that.

/!\ The exception to this rule is when we have class hierarchies outside of our control that are known not to use super() and that we want to use for diamond-shaped inheritance.

== Use of lambda, and operator.attrgetter ==

Prefer [[http://docs.python.org/release/2.5.4/lib/module-operator.html|operator]].attrgetter to `lambda`. Remember that giving functions names makes the code that calls, passes and returns them easier to debug.

== Use of open() ==

When opening a file do one of:

  1. Use an encoding (via `codecs.open(...)` in Python 2, or `open(..., encoding="...")` in Python 3), or

  2. Use `"b"` in the mode to signify reading/writing bytes.

=== Discussion of text and binary modes ===

Python 2 does implicit encoding and decoding between unicode and byte strings. This is convenient, but means we can get away with being vague when reading and writing.

Python 3, however, will not let us be vague:

{{{
$ python3
>>> open("foo", "w").write(b'bar')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: must be str, not bytes
>>> open("foo", "wb").write('bar')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'str' does not support the buffer interface
}}}

Python 3's `open()` takes a new encoding parameter - much like `codecs.open()` in Python 2 - with the following behaviour:

 "In text mode, if encoding is not specified the encoding used is platform dependent."

Equals "don't count on it". Getting into the habit of using binary mode now - or `codecs.open()` - ought to help us avoid this pitfall when porting to Python 3.

Python 3's `open()` function also enables universal newlines in text mode by default, for '''writing''' too; Python 2 does not support universal newlines when writing. Using binary mode ought to help us avoid this pitfall too.


== Use of hasattr ==
Use `safe_hasattr` from `lazr.restful.utils` instead of the built-in `hasattr` function because the latter swallows exceptions.

== Database related ==

=== Storm ===

We use two database ORM (object-relational mapper) APIs in Launchpad, the older
and deprecated SQLObject API and the new and improved [[http://storm.canonical.com|Storm]] API. All new code should use the Storm API, and you are encourages to convert existing code to Storm as part of your tech-debt payments.

Note:

 * The SQLObject and Storm `ResultSet` interfaces are not compatible, so e.g. if you need to `UNION` between these two, you will run into trouble. We are looking into ways to address this.

=== Field attributes ===

When you need to add ID attributes to your database class, use `field_id` as the attribute name instead of `fieldID`.

=== Multi-line SQL ===

SQL doesn't care about whitespace, so use triple quotes for large SQL queries or fragments, e.g.:

{{{
#!python
    query = """
        SELECT TeamParticipation.team, Person.name, Person.displayname
        FROM TeamParticipation
        INNER JOIN Person ON TeamParticipation.team = Person.id
        WHERE TeamParticipation.person = %s
        """ % sqlvalues(personID)
}}}

This also easy to cut-and-paste into `psql` for interactive testing, unlike if you use several lines of single quoted strings.


== Creating temporary files ==

We should use the most convenient method of the `tempfile` module, never taint '/tmp/' or any other 'supposed to be there' path.

Despite of being developed and deployed on Ubuntu systems, turning it into restriction might not be a good idea.

When using `tempfile.mkstemp` remember it returns an open file-descriptor which has to be closed or bound to the open file, otherwise they will leak and eventually hit the default Linux limit (1024).

There are 2 good variations according to the scope of the temporary file.

{{{
#!python
    fd, filename = mkstemp()
    os.close(fd)
    ...
    act_on_filename(filename)
}}}

Or:

{{{
#!python
    fd, filename = mkstemp()
    temp_file = os.fdopen(fd, 'w')
    ...
    temp_file.write('foo')
    temp_file.close()
}}}

'''Never''' use:

{{{
#!python
    fd, filename = mkstemp()
    temp_file = open(filename)
    temp_file.write('foo')
    temp_file.close()
    # BOOM! 'fd' leaked.
}}}

It's also important to mention that in testing context, specially if you are using the `lp.testing.TestCase` (or one of its specializations) you can simply create a brand new temporary directory (using `mkdtemp`). Create as many files you need within it and register a `cleanup` function to purge the temporary directory recursively.

{{{
#!python
class TestFoo(TestCase):
...
    def test_foo(self):
        tempdir = mkdtemp()
        self.addCleanup(shutils.rmtree, tempdir)
 ...
 do_something(os.path.join(tempdir, 'test.log'))
 ...
}}}

== Configuration hints ==

=== Vim ===

To make wrapping and tabs fit the above standard, you can add the following to your {{{.vimrc}}}:

{{{
autocmd BufNewFile,BufRead *.py set tw=78 ts=4 sts=4 sw=4 et
}}}

To make trailing whitespace visible:

{{{
set list
set listchars=tab:>.,trail:-
}}}

This will also make it obvious if you accidentally introduce a tab.

To make long lines show up:

{{{
match Error /\%>79v.\+/
}}}

For an even more in-depth Vim configuration, have a look at UltimateVimPythonSetup for a complete vim file you can copy to your local setup.

=== Emacs ===

There are actually two Emacs Python modes. Emacs comes with `python.el` which
(IMO) has some quirks and does not seem to be as popular among hardcore Python
programmers. [[http://launchpad.net/python-mode|python-mode.el]] comes with
XEmacs and is supported by a group of hardcore Python programmers. Even
though it's an add-on, it works with Emacs just fine.		 #refresh 0 https://launchpad.readthedocs.io/en/latest/guides/python.html

PythonStyleGuide (last edited 2021-11-25 16:16:21 by cjwatson)