DevNotes_DevGuide_CodingRules

SasView Coding Rules and Practices

These SasView coding principles were developed from existing SasView practice, discussions at SasView meetings/workshops, and other like projects such as Mantid ​http://www.mantidproject.org/Coding_Standards and rely heavily on general best practices and PEP recommendations. The hope is that new contributions will adhere to these as much as possible and that Pull Request reviews will be mindful of these as well.

The following document is divided into subsections (Some of these needs to be written)

• Golden Rule
• Coding Standards
  • Docstring
  • Naming
  • Imports
  • Exception Handling
  • Coding Style

• Testing
• GIT
• Code reviews
• Code Editors
• API interface
• GUI guidelines (to be included in the future)

Golden Rule:

The committed code should NEVER break the build. If you do break the build - the no. 1 priority is to fix it IMMEDIATELY.

Coding Standards

Docstring

When in doubt, the best is to follow PEP 257 ( ​https://www.python.org/dev/peps/pep-0257/ ). A few rules that are particularly recommended to follow are mentioned below:

• All modules should normally have docstrings, and all functions and classes exported by a module should also have docstrings.

• For consistency, always use """triple double quotes""" around docstrings. Use r"""raw triple double quotes""" if you use any backslashes in your docstrings.

• There are two forms of docstrings: one-liners and multi-line docstrings. One-liners are for really obvious cases. They should really fit on one line.

• Multi-line docstrings consist of a summary line just like a one-line docstring, followed by a blank line, followed by a more elaborate description.

• The docstring for a module should generally list the classes, exceptions and functions (and any other objects) that are exported by the module, with a one-line summary of each. (These summaries generally give less detail than the summary line in the object's docstring.) The docstring for a package (i.e., the docstring of the package's __init__.py module) should also list the modules and subpackages exported by the package.

• The docstring for a function or method should summarize its behavior and document its arguments, return value(s), side effects, exceptions raised, and restrictions on when it can be called (all if applicable). Optional arguments should be indicated. It should be documented whether keyword arguments are part of the interface.

• The docstring for a class should summarize its behavior and list the public methods and instance variables. If the class is intended to be subclassed, and has an additional interface for subclasses, this interface should be listed separately (in the docstring). The class constructor should be documented in the docstring for its __init__ method. Individual methods should be documented by their own docstring.

• Docstrings are processed by sphinx, and should use ReStructuredText formatting conventions.

• Models: when adding a new model the .py file starts with an r (for raw) and three sets of quotes to start the doc string and ends with a second set of 3 quotes.

• Models: don't add/remove the table of parameters which will get autogenerated.

• Models: add non-math figures to the sasmodels/sasmodels/models/img subdirectory and rename them to something sensible.

• Models: figure captions should be indented relative to the image tag. This allows us to, for example, change the font of the caption through CSS.

• Models: don't add the figure number since this will be generated automatically.

• Models: make sure all references to model parameters are appropriately named, and that the relationship between equation variables and model parameters are obvious.

Naming

• Use CamelCase for class names

• Use box_car with underscores separating words for variable and method names.

• Use box_car with underscores for namespaces & directories

• Use _box_car with a leading underscore for non-public methods and _CamelCase with a leading underscore for non-public classes.

• Never use __box_car__ with leading and trailing double underscores

• Avoid one-letter variable names (unless is acceptable one letter variable name or it is justified by the math in the paper)

• Don't make long variable names just to keep lint happy:

  [(value, key) for key, value in translation_dictionary.items()]
  

  is no better than:

  [(v, k) for k, v in translation_dictionary.items()]
  

  but the following is:

  [(latin, english) for english, latin in translation_dictionary.keys()]
  

• Models: for adding a new model no capitalization and thus no CamelCase in model names. If necessary use underscore to separate (i.e. barbell not BarBell or broad_peak not BroadPeak)

• Models: Don't add/Remove “model” in/from the name (i.e. barbell not BarBellModel)

• Models: When defining parameters use radius_core, sld_core names not core_radius and core_sld

Imports

• Import only one module per line:

  import sascalc
  import sasmodels
  

  not

  import sasmodels, sascalc
  

• It is OK to import multiple classes and/or methods from the same module on the same line:

  from sasmodels import barbell, cylinder
  

• Avoid importing * from any module. This is a typically a waste of time and memory. This leads to bugs as well when for example you have "from math import *" and "from numpy import *" in the same file and then decide to reorder them. It also makes for harder to understand code since you don't know where the symbols are coming from.

• from module import * is acceptable in __init__.py if module.__all__ defines the set of names being exported.

• Imports should be grouped in the following order:
  1. standard library imports
  2. related third party imports
  3. local application/library specific imports
• Put a blank line between each group of imports.

Exception Handling

• Use if and try/except block to test for errors that could happen in normal program operation
• If you catch an exception and all you do is calling pass or print, you have not dealt with the error properly
• Exceptions should not be used for flow control, unless there is a very good reason for it
• Error codes from a method should never be returned - use exceptions
• Never use a blank except: statement since it interferes with KeyboardInterrupt. At a minimum, use except Exception:.

Coding Style

When in doubt - follow PEP 8 ( ​https://www.python.org/dev/peps/pep-0008/ )

Indentation

• Use 4 spaces to indent, not tabs.
• Indent 4 spaces per nested level.

Spaces in Expressions & Statements

• Do not put spaces around parentheses, brackets, or curly braces.
• Do not put spaces before commas, colons, or semicolons.
• Put one space around operators.
• However one can use no spaces with operators of higher priority, e.g., y = a*x**2 + b*x + c .

Miscellaneous

• Limit lines to a maximum of 79 characters.

• Do not put semicolons at the end of lines.

• Avoid using magic numbers Use:

  MAX_BINS = 7
  if i < MAX_BINS:
  

  Not:

  if i < 7
  

Comments in the code

• Comments should usually be complete sentences.
• Use block comments to talk about the code immediately following the comments.
• Use inline comments (sparingly) to clarify confusing lines.

Testing

• Unit tests should test one method only. This allows you to easily identify what failed if the test fails.

• Unit tests should not be coupled together, therefore one unit test CANNOT rely on another unit test having completed first.

• Unit tests should use small and simple data sets.

• Tests should be fast, ideally really fast - certainly not more than a few seconds.

• Untestable code is a code-smell, if you can't get the code under test it probably needs refactoring.

• Weight your testing to be destructive rather than demonstrative. Destructive tests are more efficient in finding bugs.

• Use long and descriptive names for testing functions. The reason is testing functions are never called explicitly. square() or even sqr() is ok in running code, but in testing code you would have names such as test_square_of_number_2(),test_square_negative_number(). These function names are displayed when a test fails, and should be as descriptive as possible.

• Attributes of a good test: Fast, Clear, Isolated, Reliable

• Models: Include at least one test for each model and PLEASE make sure that the answer value is correct (i.e. don't blindly use the function return value to set the test results; instead check against the paper or another program).

• Models: Remember to test a model for critical values (e.g. q=0)

• For more detailed instructions and recommendations see Running and adding unit tests to SasView

GIT

• Remember to add files using git add before committing your changes.
• Have a self-explanatory commit message. "Merged with master" is not sufficient. It is a good practice to do a git log before pushing to master and check for the top message. git commit —amend can be used to fix commit message if we are not satisfied with it.
• Reference ticket numbers in commit message.
• Using git branches is encouraged when developing new features or implementing significant changes. It is also a good habit to regularly merge you branch with master as well as push it Github (as a backup).
• Name branches after a ticket number where appropriate, such as ticket-835 for ticket SasView/sasview#930 .
• When merging a branch with multiple commits use —squash or interactively rebase before pushing to master.

Code Reviews

• Code reviews by another developer are encouraged for minor functionality and required for major changes
• When developing in the branch it is easy to ask for pull requests. One can also test your code easily by pulling your branch and not messing up with master branch
• Follow the standard practices:
  1. Review no more than 400 lines of code at a time
  2. Take your time. Inspection rates should be under 500 LOC per hour
  3. Don't review code for more than 90 minutes at a time
  4. Authors should annotate source code before the review. (Annotations guide the reviewer through the changes, showing which files to look at first and defending the reason behind each code modification. Annotations should be directed at other reviewers to ease the process and provide more depth in context)
  5. Use checklists - they substantially improve results for both authors and reviewers (It's very likely that each person on your team makes the same 10 mistakes over and over. Omissions in particular are the hardest defects to find because it's difficult to review something that isn't there. Checklists are the most effective way to eliminate frequently made errors and to combat the challenges of omission finding)
  6. Remember to review related unit tests - they are as important as the code.
  7. Verify that the defects are actually fixed

Code Editors

• Tips and tricks of how to make your life easier when developing code with your favorite editor.
• Example editors used in the group are:
  1. vi
  2. PyCharm
  3. Visual Studio with PTVS

• Both PyCharm and VS allow source level debugging both of self-spawned processes and by attaching to an already running process.

API

The proposed access to the core functionality should be consistent for all perspectives and straightforward to use.

• All known input arguments should be set in __init__ , during instantiation, with proper checks
• Additional input data can be set directly through the @property decorator

Getters:

@property
def intensity(self):
    return self.wave.intensity

Setters:

@intensity.setter
def intensity(self, value):
    self.wave.set_intensity(intensity)

• All user accessible input/output data should be available through getters __getattr__ and __setattr__ should be avoided.
• Main functionality should be exposed as a method with easy to understand name, e.g. calculate() and should return the results object/structure
• When defining the interface, type hints are an important part of the definition. Type hints are described in PEP-484 ​https://www.python.org/dev/peps/pep-0484 and should be added for all API methods.

def fit(self, msg_q=None, q=None, handler=None, curr_thread=None, ftol=1.49012e-8, reset_flag=False):
       # type: (CalcThread.queue, CalcThread.queue, FitHandler, CalcThread, float, bool) -> Union[CalcThread.queue, FResult]

• In cases where several pieces of information are calculated at the same time, a list of outputs, a dictionary, or an output object that contains the computed data should be returned
• Error codes from a method should never be returned - use exceptions