Changes between Initial Version and Version 1 of DevNotes/DevGuide/CodingRules

Timestamp:

May 27, 2016 5:17:26 PM (8 years ago)

Author:

wojciech

Comment:

—

DevNotes/DevGuide/CodingRules

@@ +1 @@
+Sasview Coding Rules
+
+The coding rules are based on few already existing rules of SasView and other programs available from SINE2020 project (primarily Mantid http://www.mantidproject.org/Coding_Standards). The idea is to implement the coding rules at ESS and then potentially suggest the guideline for future developers within SASView community.
+
+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 (Not sure if we want to include it)
+
+* GUI guidelines (to be included in the future)
+
+* API interface (to be included in the future)
+
+Golden Rule:
+
+    The committed code should NEVER break the build.
+    If you do break the build - the #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.
+     
+     
+    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: wherever possible, try to use images of math functions with Latex equivalents. You can use the live demo Mathjax page ( http://www.mathjax.org/) to make sure the equation looks as expected. Also a lot of the Latex code can be taken from (or edited) from the PDF document created by Paul Kienzle:  http://sasview.org/attachment/wiki/SasModels%20Work%20Package/Equations.docx.pdf.
+    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.
+
+    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.
+
+    Imports should be grouped in the following order:
+        standard library imports
+        related third party imports
+        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
+
+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. not a random number).
+    Models: Remember to test a model for critical values (e.g. q=0)
+
+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.  
+    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).
+    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:
+        Review no more than 400 lines of code at a time
+        Take your time. Inspection rates should be under 500 LOC per hour
+        Don't review code for more than 90 minutes at a time
+        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)
+        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)
+        Remember to review related unit tests - they are as important as the code.
+        Verify that the defects are actually fixed
+
+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:
+
+    vi
+    PyCharm
+    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