In-development code style guide for OpenPathSampling projects

code_style.rst

Code Style

Code style should be mostly PEP-8.

The easiest way to get good code is to pass it through Black (with an 80 column maximum). Our code style isn't exactly Black, but Black code will almost always be acceptable, and it is easy for contributors to reformat with Black. Just install black (pip or the conda-forge channel of conda) and run the command black -l 80 $filename on your files. It will reformat them in a way that is suitable for OPS.

Note

Please do not use Black to reformat files that are already in the OPS code base. Our style is not Black, and this will lead to undesirable style changes that will need to be reverted.

For the more advanced users, we recommend using pylint and flake8 on your code. Things that these tools list as errors should almost always be handled. Things that these tools list as warnings should be considered.

Here are a few cases where we find exceptions to the rules.

flake8 errors

• E127: continuation line over-indented: When continuing from a backslash, our style is an 8-space extra indent. This distinguishes it from a regular indent, and is particularly important if the continued line is a statement that introduces a new level of indent. For non-backslash continuation, E127 should be obeyed.
• E206: too many leading '#' for block comment: We occasionally use "section separator" comments to distinguish areas within a code file that contain code for a specific topic. The format we use for this is a single line comment with 3, 5, ... initial # characters (with more characters indicating "sublevels" of the section). The line is exactly 76 charaters wide, and is padded with # at the end. Flake8 misidentifies these as block comments and raises the E206 error. However, the actual block comments should have a single # and should not raise E206.
• E731: do not assign a lambda expression: In many cases this is good advice. If a function is likely to be reused, better to have it as a function. Even if not, it might be useful to provide it as a nested function, which would make it easier to change the implementation in the future (for example, to add logging). However, there are some cases where assigning a lambda makes sense. For example, if a class __init__ method takes callables, using None as a default value in the signature and then assigning to a lambda is completely reasonable.

pylint warnings

• logging-fstring-interpolation: We generally prefer log statements to be formatted as f-strings because they are easier to read. However, if you find that you are logging in a performance-critical section of code, you should use % formatting (and you may consider wrapping logging blocks in an if logger.isEnabledFor(LEVEL) guard).

• missing-function-docstring: Very simple functions, private functions, and property-decorated functions may sometimes ignore missing docstrings. Similarly, "dunder" methods (if they add no unusual functionality) don't need them. But just about everything else does.

• too-few-public-methods: Consider this, but consider it briefly. In many cases, this is acceptable. Among other things, it faciliates the extension to future functionality. For example, this frequently occurs if we have use a callable class to create a function that also encapsulates some data. The same could be done by either creating a more complicated function and using functools.partial or by creating a factory function that fixes some parameters. However, both of those approaches are less convenient for introspection and extension, so a callable class is often preferred.

• raise-missing-from: This is rarely actually useful. In Python 3, the raise ... from e behavior is implicit if you raise an exception within an except block. raise ... from is only necessary if you want to change the default behavior (e.g., raise ... from None).

• invalid-name: For the most part, we strongly recommend following the rule that you use snake_case variable names, with names at least 3 letters long. However, there are certain exceptions to the rule:

  • Variables in scientific equations. You can use single letters and capital letters as appropriate -- in context, a name V is just as clear as potential_energy, and makes equations read more naturally.

  • Single-letters in comprehensions. In areas of limited scope, such as list comprehensions, it is acceptable to use single letters (for example, [a.index for a in atoms] is perfectly readable).

  • Special cases for single-letter names in ``as`` clauses. Specifically, the following are permitted:

    • except ... as e
    • with open(...) as f

    As with comprehesions, the principle here is that the scope is limited. Furthermore, these are relatively common conventions.

  • OPS-specific special cases for short variable names: The following variable names are allowed, even through they are fewer than 3 letters:

    • cv to represent a collective variable

Code style for tests

Most of the code style remains the same for tests, however, there are a few errors/warnings that we allow in test code even though they are forbidden in the main code. This is mainly because we handle imports differently than in the core code.

• F403: 'import *' used [flake8] / wildcard-import [pylint] / unused-wildcard-import / F405: defined from star imports: Many test modules map one-to-one with a code module; that is, for a module module.py, the tests are located in test_module.py. In this case, we allow wildcard import of module.py within test_module.py. The principle is that test_module.py is, in a way, an extension of module.py, and everything available in module.py should be available in test_module.py. Note that only this module is allowed to have a wildcard import.
• wrong-import-order

For the CLI

• import-outside-toplevel: The OPS 1.0 import is slow. Because of this, in the CLI we delay import of anything from OPS until it is needed. Otherwise, simple interactions such as using the --help functionality or even getting tab completion would have to wait for the OPS load. The long-term goal will be to speed up the OPS import time, but for now we recommend delaying imports.