Basic concepts:

  • Use PyLint as the verification tool for code style
  • Start with base rules, and add additional ones every time we run into any ambiguous scenarios
  • Aspire to only support rules which can be verified automatically
  • Upon PyLint new release, consider the new base rules and possibly add new exceptions rather than changing the code to fit the new base rules

List of pylint builtin messages (rules): http://pylint-messages.wikidot.com/all-messages

 

List of additional rules:

  1. Line width should be capped at 100 rather than the default 79
  2. Exceptions
    1. Exceptions are a means of breaking out of the normal flow of control of a code block to handle errors or other exceptional conditions.
    2. Never use catch-all except: statements, or catch Exception or StandardError, unless you are re-raising the exception or in the outermost block in your thread (and printing an error message). Python is very tolerant in this regard and except: will really catch everything including misspelled names, sys.exit() calls, Ctrl+C interrupts, unittest failures and all kinds of other exceptions that you simply don't want to catch.
  3. Power Features

    1. It's very tempting to use these "cool" features when they're not absolutely necessary. It's harder to read, understand, and debug code that's using unusual features underneath. It doesn't seem that way at first (to the original author), but when revisiting the code, it tends to be more difficult than code that is longer but is straightforward.
  4. Classes
    1. If a class inherits from no other base classes, explicitly inherit from object. This also applies to nested classes.
  5. Imports
    1. Import modules and (and not attributes).
    2. Use relative import when possible (while maintaining readability).
    3. When importing several modules from the same package always use parenthesis, while breaking lines between each module.

    4. When importing several attributes from the same module, the import should be on the same line, separated by commas. 
    5. The import section should hold 3 (4 - optional) subsection:
      1. standard libs
      2. 3d party libs
      3. remote aria packages (if for readability reasons, it is better to import directly from aria, and not a relative path). - This is optional.
      4. aria packages (while inside this section the order should be according to the relative import 'distance' - that is, the closer the actual module is, the lower it will be. In effect ... will come before ..). 

      in each subsection, `import X` should come before from A import B, while inside each import and from . import . the order should be according to the length of the line

      for example:

      import time
      from datetime import datetime
      from contextlib import contextmanager
      
      import networkx
      
      from aria import (
          events,
      	logger,
      )
      from aria.workflows.core.task import BaseTask, OperationTask
      
      from ...storage import models
      from .. import exceptions
      from . import translation
      from . import task as engine_task

       

       

  6. Naming  
    • if the variable is not a part of the public API, use ‘_’ as a prefix to the variables name.
    • single_trailing_underscore_: used by convention to avoid conflicts with Python keyword, e.g. class_
  7. between class declaration and any following member should be exactly one empty line
  8. When creating a mixin class, the name should end with ‘mixin’ (not case sensitive) - this will enable ignoring missing members.

TODO: attach an updated PyLint configuration file


 

  • No labels

3 Comments

  1. There are a few issues to mention with imports:

    c.

    The line break seems very verbose to me, especially with the extra comma and extra line for closing parenthesis. Could we possibly allow things like these:

    from aria import (events, logger)
    
    from .definitions import (PropertyDefinition, AttributeDefinition, ParameterDefinition,
                              OperationDefinition, InterfaceDefinition, RelationshipDefinition,
                              RequirementDefinition, CapabilityDefinition)

    Note that if we do start the list on the first line, then pylint will insist that indentation for the next lines should agree with the first.

    Also, no standard order is mentioned for items. Should they be alphabetical? What about upper/lowercase? Constants before classes before functions?

    d.iii. Actually, the reason to import from `aria` instead of relative imports is not just for readability. From within ARIA extensions, you have to assume that they can be installed as a separate package (with their own `setup.py`). In that case, relative imports will not work.

    d.iv Having the order be the length of the line is very awkward. At the minimum we should be able to group them by module prefixes. For example:

    from aria import dsl_specification
    from aria.utils import FrozenDict, FrozenList, cachedmethod
    from aria.presentation import (has_fields, primitive_field, primitive_list_field, object_field,
                                   object_list_field, object_dict_field, object_sequenced_list_field,
                                   field_validator, type_validator, list_type_validator)
    
    from .misc import Description, MetaData, Repository, Import, SubstitutionMappings
    from .definitions import ParameterDefinition
    from .assignments import (PropertyAssignment, AttributeAssignment, RequirementAssignment,
                              CapabilityAssignment, InterfaceAssignment, ArtifactAssignment)
    from .types import (ArtifactType, DataType, CapabilityType, InterfaceType, RelationshipType,
                        NodeType, GroupType, PolicyType)
    from .filters import NodeFilter
    from .presentation.extensible import ExtensiblePresentation
    from .presentation.field_validators import copy_validator, policy_targets_validator
    from .presentation.types import (convert_shorthand_to_full_type_name,
                                     get_type_by_full_or_shorthand_name)
    from .modeling.properties import get_assigned_and_defined_property_values, get_parameter_values
    from .modeling.interfaces import get_template_interfaces
    from .modeling.requirements import get_template_requirements
    from .modeling.capabilities import get_template_capabilities
    from .modeling.artifacts import get_inherited_artifact_definitions
    from .modeling.policies import get_policy_targets
    from .modeling.copy import get_default_raw_from_copy

    Also, I'm not really sure length of line would have much meaning if some these are multi-line imports.

    My suggestion is that within each sub-section we should generally order alphabetically. This would apply to sections i. ii. and iii., too. This is also what a lot of automatic tools would do.

  2. Suggestion for Sphinx docstrings:

    It is possible to split URLs into multiple lines without special notation. Let's by convention split before / and # where possible, like so:

        """
        An Artifact Type is a reusable entity that defines the type of one or more files that are used
        to define implementation or deployment artifacts that are referenced by nodes or relationships
        on their operations.
    
        See the `TOSCA Simple Profile v1.0 cos01 specification <http://docs.oasis-open.org/tosca
        /TOSCA-Simple-Profile-YAML/v1.0/cos01/TOSCA-Simple-Profile-YAML-v1.0-cos01.html
        #DEFN_ENTITY_ARTIFACT_TYPE>`__
        """
    • Well, it looks like you've imported an attribute (class), import a module could save the import section in your first example.
    • The alphabetical ordering doesn't really contribute anything, you wouldn't really search for a package/module name by alphabetical order in the imports.