Context Navigation

Coding standards

Here are some Python code style guidelines. We also include official Python guidelines by reference (see the section on "official Python standards" below). This document overrides the official standards whenever there is a conflict.

Basic standards

Compatibility

Tahoe requires Python v2.7.13 or greater (although the current code only refuses to run on Python < 2.7). No effort should be made to offer compatibility with versions of Python older than 2.7.13. Effort should be made to work with every Python release from v2.7.13 to the most recent 2.x, inclusive.

Naming and Layout

Use underscore_separated_names for functions, UpperCamelCapNames for classes, alllowercasenames for modules, and ALL_CAPS_NAMES for constants. Use all lower-case variable names (e.g. variable_name or variablename). Prepend a leading underscore to private names.
Put parentheses around tuples if it helps make the code more readable, leave them off if not.

Comments, Idioms, Miscellany, License, Imports, Docstrings, Line widths

Here is a useful header for starting new Python files:

"""
Optional doc string describing the module here.
"""

# import Python Standard Library modules here

from allmydata.util.assertutil import _assert, precondition, postcondition

# import from other libraries, with a blank line between each library

# your code here

Do not include a per-file copyright header (they just get out of date and are redundant with README.rst ).
Put two blank lines between classes.
Put one blank line before a block comment if the preceding line is code at the same indent level (this makes it harder to mistake the code as part of the comment, and makes the comment easier to read).
Feel free to ignore the part of PEP-8 that says to put each module import on a separate line, but don't import modules from multiple separate packages on the same line.
Ignore the part of PEP-257 which says to put the trailing """ of a multi-line docstring on a separate line separated by a blank line. (That rule appears to have been motivated by a limitation of Emacs which has been fixed.)
Ignore the part of PEP-8 which specifes 79- or 72- char line widths. Lines should preferably be less than 100 columns, but we don't enforce this strictly. It is more important to break lines at points that are natural for readability than to follow a fixed line width restriction. Where possible, continuation lines should be indented as far as necessary to make them match up with the subexpression (e.g. argument list) they belong to.
PEP 8 says: "If operators with different priorities are used, consider adding whitespace around the operators with the lowest priority(ies)." This is a good rule; note that it also applies to some non-obvious low-priority operators, like ':' for list slicing. (Example: a[b-c : d] good, a[b - c:d] bad. If a slice is from the start or to the end of the array, put the ':' immediately next to the bracket on that side.)

Truths and Falsehoods

Don't use the literals True or False in conditional expressions -- instead just write the expression which will evaluate to true or false. For example, write if expr: instead of if expr == True: and if not expr: instead of if expr == False:.
Avoid relying on the fact that empty sequences, empty strings, empty dicts, 0, and None are treated as false. Write if len(items) == 0:, if thing is None:, etc.

Miscellaneous

athing in adict.keys() can be replaced by athing in adict, which evaluates to the same boolean result, but is more succinct and more efficient at run-time.

Idioms

Preconditions and Assertions

Basic preconditions and assertions

Make sure you have from allmydata.util.assertutil import _assert, precondition, postcondition in your imports (as shown in the template above). Now design preconditions for your methods and functions, and assert them like this:

def oaep(m, emLen, p=""):
    precondition(emLen >= (2 * SIZE_OF_UNIQS) + 1, "emLen is required to be big enough.", emLen=emLen, SIZE_OF_UNIQS=SIZE_OF_UNIQS)
    ...

Notice how you pass in any values that ought to be printed out in the error message if the assertion fails. In the example, we pass the values emLen and SIZE_OF_UNIQS. You can pass these as normal args or keyword args. If you use keyword args then the name of the argument will also appear in the error message, which can be helpful. For example, if the assertion above fails, then a debug message will appear at the end of the stack trace, like this:

>>> oaep("some secret thingie", 20)
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "<stdin>", line 2, in oaep
  File "/home/zooko/playground/pyutil/pyutil/assertutil.py", line 47, in precondition
    raise preconditionfailureexception
AssertionError: precondition: emLen is required to be big enough. -- emLen: 20 <type 'int'>, 'SIZE_OF_UNIQS': 20 <type 'int'>

The "error message" that will accompany a failed expression should be a statement of what is required for correct operation. Don't write something like "Spam isn't firm.", because that is ambiguous: the error could be that the spam is supposed to be firm and it isn't, or the error could be that spam isn't supposed to be firm and it is! It helps to use the words "required to" in your message, for example "Spam is required to be firm.".

Assertions are not a substitute for proper error handling! An assertion, precondition or postcondition should only be used for cases that "cannot happen" unless the code is incorrect. They should not be used to check for invalid inputs; in that case, raise an exception instead.

Avoid "bare assert"

Python's built-in assert statement, unlike allmydata.util.assertutil._assert, can be switched off by the -O option to python or the PYTHONOPTIMIZE environment variable. Although this might sound useful to reduce the overhead of assertions, in practice that overhead is negligable, and conditional assertions are more trouble than they're worth (partly because they create a configuration that is mostly untested). We are in the process of removing all bare asserts from the codebase (#1968).

Class invariants

If your class has internal state which is complicated enough that a bug in the class's implementation could lead to garbled internal state, then you should have a class invariant. A class invariant is a method like this (an actual example from BlockWrangler, but truncated for space):

def _assert_invariants(self):
    # All of the keys in all of these dicts are required to be ids.
    for d in (self.bId2chunkobj, self.bId2peers, self.Idsofwantedblocks, self.Idsoflocatedblocks,):
        _assert(not [key for key in d.keys() if not idlib.is_id(key)], "All of the keys in these dicts are required to be ids.", listofnonIds=[key for key in d.keys() if not idlib.is_id(key)])

    # For each (peer, blockId,) tuple in peerclaimedblock, if the peer *has*
    # claimed the block, then the blockId is required to appear in bId2peers[blockId],
    # and if the peer has claimed *not* to have the block then the blockId
    # is required *not* to appear in bId2peers[blockId].
    for ((peer, blockId,), claim,) in self.peerclaimedblock.items():
        _assert((claim == "yes") == (peer in self.bId2peers.get(blockId, ())), "The blockId must appear in bId2peers if and only if the peer has claimed the block.", claim=claim, peer=peer, bId2peersentry=self.bId2peers.get(blockId, ()))

Now you can put assert self._assert_invariants() everywhere in your class where the class ought to be in an internally consistent state. For example, at the beginning of every externally-callable method. This technique can be very valuable in developing a complex class -- it catches bugs early, it isolates bugs into specific code paths, and it clarifies the internal structure of the class so that other developers can hack on it without subtle misunderstandings.

We actually appear to only have one instance of this pattern in Tahoe at time of writing, in allmydata.util.dictutil. It has the disadvantage of cluttering up the logic with calls to _assert_invariants, and should probably be used sparingly. -- Daira

Assertion policy

One axis of interest is how time-consuming the checks are. Many precondition checks can cause typical runtime to explode to O(n²) or O(n³), for example SortedList.__contains__ called _assert_invariants which took O(n log n) each time, when __contains__ ought to be O(log n). A caller who was expecting if b in list to take O(log n) could easily wind up turning their O(n log n) routine into O(n²) or worse.

Another axis is "who could cause it to fail": some checks are looking only at internal state. For example, if SortedList._assert_invariants fails, it indicates a problem in some SortedList method. Other checks are enforcing the external API, like those which do typechecks on input arguments. Even after the SortedList developer has gained confidence in the code and decides that internal checks are no longer necessary, it may be useful to retain the external checks to isolate usage problems that exist in callers.

The general rule is that assertions must not prevent nodes from being functional, even for heavy traffic.

Time-consuming internal checks: once the code is working properly, consider removing them, or making them conditional on a module-level DEBUG constant.

Cheap internal checks: keep them even after the code is working properly.

External checks: keep them if they are necessary for security or robustness (but probably by raising an exception rather than as an assertion). If they are not necessary for security or robustness, treat in the same way as internal checks.

Configuration

Minimizing configuration

Do not implement configuration files for modules or libraries -- code that is going to be used by other code. Only applications -- code that is going to be used by humans -- have configuration files. Modules and libraries get "configured" by the code that calls them, for example by passing arguments to their constructors.
If there are constant values which end-users do not need to modify, then do not make them configurable, but put them in all-caps variables at the beginning of the Python file in which they are used.
Design algorithms so that they have as few "voodoo constants" and "tweakable parameters" as possible.

How to implement configuration

Whether in application code or in library code, never pass configuration values via a configuration object. Instead use Python parameters. For example -- here's another real-life example -- do not write

class BlockStore:
    def __init__(self, confdict={}, recoverdb=True, name='*unnamed*'):
        if confdict.has_key('MAX_MEGABYTES'):
            self.maxspace = (2**20) * int(confdict.get('MAX_MEGABYTES'))
        else:
            self.maxspace = None
        self.basepath = os.path.abspath(confdict.get("PATH", ""))
        self.maintainertype = confdict.get("MAINTAINER", "rnd").lower()
        self.backendtype = confdict.get("BACKEND", "flat").lower()

, but instead write

class BlockStore:
    def __init__(self, maxspace=None, path="", maintainertype="rnd", backendtype="flat", recoverdb=True, name='*unnamed*'):
        self.basepath = os.path.abspath(path)
        self.maintainertype = maintainertype
        self.backendtype = backendtype

Official Python Standards

These are listed in decreasing order of priority, so if a point in one of the latter guidelines contradicts a point in one of the earlier ones, then go with the earlier. The Tahoe-LAFS-specific guidelines above override all else, of course.

Last modified at 2019-03-15T17:07:37Z Last modified on 2019-03-15T17:07:37Z

Download in other formats:

Plain Text