Merge lp:~zyga/checkbox/fix-1444996 into lp:checkbox

[Zygmunt Krynicki]

d66ee14 plainbox:pod: fix PEP257 issues
47ab7c6 plainbox:pod: fix PEP-8 issues
b8e68dd plainbox:pod: add .counter to each Field.
836d41c plainbox:pod: use field.counter to determine field ordering
f8b9993 plainbox:pod: move POD methods to new PODBase class
6470a6a plainbox:pod: add @podify decorator
5d52f5b plainbox:pod: refresh __all__

[Maciej Kisielewski (kissiel)]

Good stuff, +1

review: Approve

=== modified file 'plainbox/plainbox/impl/pod.py'
--- plainbox/plainbox/impl/pod.py	2015-03-27 20:26:58 +0000
+++ plainbox/plainbox/impl/pod.py	2015-04-16 14:45:30 +0000
@@ -16,8 +16,10 @@
 # You should have received a copy of the GNU General Public License
 # along with Checkbox.  If not, see <http://www.gnu.org/licenses/>.
 """
-:mod:`plainbox.impl.pod` -- Plain Old Data
-==========================================
+Plain Old Data.
+
+:mod:`plainbox.impl.pod`
+========================
 
 This module contains the :class:`POD` and :class:`Field` classes that simplify
 creation of declarative struct-like data holding classes. POD classes get a
@@ -63,26 +65,28 @@
 from plainbox.i18n import gettext as _
 from plainbox.vendor.morris import signal
 
-__all__ = ['POD', 'Field', 'MANDATORY', 'UNSET', 'read_only_assign_filter',
-           'type_convert_assign_filter', 'type_check_assign_filter',
-           'modify_field_docstring']
+__all__ = ('POD', 'PODBase', 'podify', 'Field', 'MANDATORY', 'UNSET',
+           'read_only_assign_filter', 'type_convert_assign_filter',
+           'type_check_assign_filter', 'modify_field_docstring')
 
 
 _logger = getLogger("plainbox.pod")
 
 
 class _Singleton:
-    """
-    A simple object()-like singleton that has a more useful repr()
-    """
+
+    """ A simple object()-like singleton that has a more useful repr(). """
 
     def __repr__(self):
         return self.__class__.__name__
 
 
 class MANDATORY(_Singleton):
+
     """
-    Singleton that can be used as a value in :attr:`Field.initial`.
+    Class for the special MANDATORY object.
+
+    This object can be used as a value in :attr:`Field.initial`.
 
     Using ``MANDATORY`` on a field like that makes the explicit initialization
     of the field mandatory during POD initialization. Please use this value to
@@ -95,7 +99,10 @@
 
 
 class UNSET(_Singleton):
+
     """
+    Class of the special UNSET object.
+
     Singleton that is implicitly assigned to the values of all fields during
     POD initialization. This way all fields will have a value, even early at
     the time a POD is initialized. This can be important if the POD is somehow
@@ -109,6 +116,7 @@
 
 
 class Field:
+
     """
     A field in a plain-old-data class.
 
@@ -190,8 +198,11 @@
     assigned to the POD.
     """
 
+    _counter = 0
+
     def __init__(self, doc=None, type=None, initial=None, initial_fn=None,
                  notify=False, notify_fn=None, assign_filter_list=None):
+        """ Initialize (define) a new POD field. """
         self.__doc__ = dedent(doc) if doc is not None else None
         self.type = type
         self.initial = initial
@@ -210,15 +221,16 @@
             self.__doc__ += (
                 '\n\nSide effects of assign filters:\n'
                 + '\n'.join('  - {}'.format(extra) for extra in doc_extra))
+        self.counter = self.__class__._counter
+        self.__class__._counter += 1
 
     def __repr__(self):
+        """ Get a debugging representation of a field. """
         return "<{} name:{!r}>".format(self.__class__.__name__, self.name)
 
     @property
     def is_mandatory(self) -> bool:
-        """
-        Flag indicating if the field needs a mandatory initializer.
-        """
+        """ Flag indicating if the field needs a mandatory initializer. """
         return self.initial is MANDATORY
 
     def gain_name(self, name: str) -> None:
@@ -252,7 +264,8 @@
         assert self.signal_name is not None
         if not hasattr(cls, self.signal_name):
             signal_def = signal(
-                self.notify_fn if self.notify_fn is not None else self.on_changed,
+                self.notify_fn if self.notify_fn is not None
+                else self.on_changed,
                 signal_name='{}.{}'.format(cls.__name__, self.signal_name))
             setattr(cls, self.signal_name, signal_def)
 
@@ -312,8 +325,121 @@
                       self.signal_name, old, new)
 
 
+@total_ordering
+class PODBase:
+
+    """ Base class for POD-like classes. """
+
+    field_list = []
+    namedtuple_cls = namedtuple('PODBase', '')
+
+    def __init__(self, *args, **kwargs):
+        """
+        Initialize a new POD object.
+
+        Positional arguments bind to fields in declaration order. Keyword
+        arguments bind to fields in any order but fields cannot be initialized
+        twice.
+
+        :raises TypeError:
+            If there are more positional arguments than fields to initialize
+        :raises TypeError:
+            If a keyword argument doesn't correspond to a field name.
+        :raises TypeError:
+            If a field is initialized twice (first with positional arguments,
+            then again with keyword arguments).
+        :raises TypeError:
+            If a ``MANDATORY`` field is not initialized.
+        """
+        field_list = self.__class__.field_list
+        # Set all of the instance attributes to the special UNSET value, this
+        # is useful if something fails and the object is inspected somehow.
+        # Then all the attributes will be still UNSET.
+        for field in field_list:
+            setattr(self, field.instance_attr, UNSET)
+        # Check if the number of positional arguments is correct
+        if len(args) > len(field_list):
+            raise TypeError("too many arguments")
+        # Initialize mandatory fields using positional arguments
+        for field, field_value in zip(field_list, args):
+            setattr(self, field.name, field_value)
+        # Initialize fields using keyword arguments
+        for field_name, field_value in kwargs.items():
+            field = getattr(self.__class__, field_name, None)
+            if not isinstance(field, Field):
+                raise TypeError("no such field: {}".format(field_name))
+            if getattr(self, field.instance_attr) is not UNSET:
+                raise TypeError(
+                    "field initialized twice: {}".format(field_name))
+            setattr(self, field_name, field_value)
+        # Initialize remaining fields using their default initializers
+        for field in field_list:
+            if getattr(self, field.instance_attr) is not UNSET:
+                continue
+            if field.is_mandatory:
+                raise TypeError(
+                    "mandatory argument missing: {}".format(field.name))
+            if field.initial_fn is not None:
+                field_value = field.initial_fn()
+            else:
+                field_value = field.initial
+            setattr(self, field.name, field_value)
+
+    def __repr__(self):
+        """ Get a debugging representation of a POD object. """
+        return "{}({})".format(
+            self.__class__.__name__,
+            ', '.join([
+                '{}={!r}'.format(field.name, getattr(self, field.name))
+                for field in self.__class__.field_list]))
+
+    def __eq__(self, other: "POD") -> bool:
+        """
+        Check that this POD is equal to another POD.
+
+        POD comparison is implemented by converting them to tuples and
+        comparing the two tuples.
+        """
+        if not isinstance(other, POD):
+            return NotImplemented
+        return self.as_tuple() == other.as_tuple()
+
+    def __lt__(self, other: "POD") -> bool:
+        """
+        Check that this POD is "less" than an another POD.
+
+        POD comparison is implemented by converting them to tuples and
+        comparing the two tuples.
+        """
+        if not isinstance(other, POD):
+            return NotImplemented
+        return self.as_tuple() < other.as_tuple()
+
+    def as_tuple(self) -> tuple:
+        """
+        Return the data in this POD as a tuple.
+
+        Order of elements in the tuple corresponds to the order of field
+        declarations.
+        """
+        return self.__class__.namedtuple_cls(*[
+            getattr(self, field.name)
+            for field in self.__class__.field_list
+        ])
+
+    def as_dict(self) -> dict:
+        """ Return the data in this POD as a dictionary. """
+        return {
+            field.name: getattr(self, field.name)
+            for field in self.__class__.field_list
+        }
+
+
 class _FieldCollection:
+
     """
+    Support class for constructing POD meta-data information.
+
     Helper class that simplifies :class:`PODMeta` code that harvests
     :class:`Field` instances during class construction. Looking at the
     namespace and a list of base classes come up with a list of Field objects
@@ -330,8 +456,15 @@
         self.field_list = []
         self.field_origin_map = {}  # field name -> defining class name
 
+    def inspect_cls_for_decorator(self, cls: type) -> None:
+        """ Analyze a bare POD class. """
+        self.inspect_base_classes(cls.__bases__)
+        self.inspect_namespace(cls.__dict__, cls.__name__)
+
     def inspect_base_classes(self, base_cls_list: "List[type]") -> None:
         """
+        Analyze base classes of a POD class.
+
         Analyze a list of base classes and check if they have consistent
         fields.  All analyzed fields are added to the internal data structures.
 
@@ -339,7 +472,7 @@
             A list of classes to inspect. Only subclasses of POD are inspected.
         """
         for base_cls in base_cls_list:
-            if not issubclass(base_cls, POD):
+            if not issubclass(base_cls, PODBase):
                 continue
             base_cls_name = base_cls.__name__
             for field in base_cls.field_list:
@@ -347,6 +480,8 @@
 
     def inspect_namespace(self, namespace: dict, cls_name: str) -> None:
         """
+        Analyze namespace of a POD class.
+
         Analyze a namespace of a newly (being formed) class and check if it has
         consistent fields. All analyzed fields are added to the internal data
         structures.
@@ -354,15 +489,19 @@
         .. note::
             This method calls :meth:`Field.gain_name()` on all fields it finds.
         """
+        fields = []
         for field_name, field in namespace.items():
             if not isinstance(field, Field):
                 continue
             field.gain_name(field_name)
+            fields.append(field)
+        fields.sort(key=lambda field: field.counter)
+        for field in fields:
             self.add_field(field, cls_name)
 
     def get_namedtuple_cls(self, name: str) -> type:
         """
-        Create a new namedtuple that corresponds to the fields seen so far
+        Create a new namedtuple that corresponds to the fields seen so far.
 
         :parm name:
             Name of the namedtuple class
@@ -393,6 +532,7 @@
 
 
 class PODMeta(type):
+
     """
     Meta-class for all POD classes.
 
@@ -415,6 +555,8 @@
     @classmethod
     def __prepare__(mcls, name, bases, **kwargs):
         """
+        Get a namespace for defining new POD classes.
+
         Prepare the namespace for the definition of a class using PODMeta as a
         meta-class. Since we want to observe the order of fields, using an
         OrderedDict makes that task trivial.
@@ -422,8 +564,28 @@
         return OrderedDict()
 
 
+def podify(cls):
+    """
+    Decorator for POD classes.
+
+    The decorator offers an alternative from using the POD class (with the
+    PODMeta meta-class). Instead of using that, one can use the ``@podify``
+    decorator on a PODBase-derived class.
+    """
+    if not isinstance(cls, type) or not issubclass(cls, PODBase):
+        raise TypeError("cls must be a subclass of PODBase")
+    fc = _FieldCollection()
+    fc.inspect_cls_for_decorator(cls)
+    cls.field_list = fc.field_list
+    cls.namedtuple_cls = fc.get_namedtuple_cls(cls.__name__)
+    for field in fc.field_list:
+        field.alter_cls(cls)
+    return cls
+
+
 @total_ordering
-class POD(metaclass=PODMeta):
+class POD(PODBase, metaclass=PODMeta):
+
     """
     Base class that removes boilerplate from plain-old-data classes.
 
@@ -449,111 +611,11 @@
     uses that type.
     """
 
-    def __init__(self, *args, **kwargs):
-        """
-        Initialize a new POD object.
-
-        Positional arguments bind to fields in declaration order. Keyword
-        arguments bind to fields in any order but fields cannot be initialized
-        twice.
-
-        :raises TypeError:
-            If there are more positional arguments than fields to initialize
-        :raises TypeError:
-            If a keyword argument doesn't correspond to a field name.
-        :raises TypeError:
-            If a field is initialized twice (first with positional arguments,
-            then again with keyword arguments).
-        :raises TypeError:
-            If a ``MANDATORY`` field is not initialized.
-        """
-        field_list = self.__class__.field_list
-        # Set all of the instance attributes to the special UNSET value, this
-        # is useful if something fails and the object is inspected somehow.
-        # Then all the attributes will be still UNSET.
-        for field in field_list:
-            setattr(self, field.instance_attr, UNSET)
-        # Check if the number of positional arguments is correct
-        if len(args) > len(field_list):
-            raise TypeError("too many arguments")
-        # Initialize mandatory fields using positional arguments
-        for field, field_value in zip(field_list, args):
-            setattr(self, field.name, field_value)
-        # Initialize fields using keyword arguments
-        for field_name, field_value in kwargs.items():
-            field = getattr(self.__class__, field_name, None)
-            if not isinstance(field, Field):
-                raise TypeError("no such field: {}".format(field_name))
-            if getattr(self, field.instance_attr) is not UNSET:
-                raise TypeError(
-                    "field initialized twice: {}".format(field_name))
-            setattr(self, field_name, field_value)
-        # Initialize remaining fields using their default initializers
-        for field in field_list:
-            if getattr(self, field.instance_attr) is not UNSET:
-                continue
-            if field.is_mandatory:
-                raise TypeError(
-                    "mandatory argument missing: {}".format(field.name))
-            if field.initial_fn is not None:
-                field_value = field.initial_fn()
-            else:
-                field_value = field.initial
-            setattr(self, field.name, field_value)
-
-    def __repr__(self):
-        return "{}({})".format(
-            self.__class__.__name__,
-            ', '.join([
-                '{}={!r}'.format(field.name, getattr(self, field.name))
-                for field in self.__class__.field_list]))
-
-    def __eq__(self, other: "POD") -> bool:
-        """
-        Check that this POD is equal to another POD
-
-        POD comparison is implemented by converting them to tuples and
-        comparing the two tuples.
-        """
-        if not isinstance(other, POD):
-            return NotImplemented
-        return self.as_tuple() == other.as_tuple()
-
-    def __lt__(self, other: "POD") -> bool:
-        """
-        Check that this POD is "less" than an another POD.
-
-        POD comparison is implemented by converting them to tuples and
-        comparing the two tuples.
-        """
-        if not isinstance(other, POD):
-            return NotImplemented
-        return self.as_tuple() < other.as_tuple()
-
-    def as_tuple(self) -> tuple:
-        """
-        Return the data in this POD as a tuple.
-
-        Order of elements in the tuple corresponds to the order of field
-        declarations.
-        """
-        return self.__class__.namedtuple_cls(*[
-            getattr(self, field.name)
-            for field in self.__class__.field_list
-        ])
-
-    def as_dict(self) -> dict:
-        """
-        Return the data in this POD as a dictionary
-        """
-        return {
-            field.name: getattr(self, field.name)
-            for field in self.__class__.field_list
-        }
-
 
 def modify_field_docstring(field_docstring_ext: str):
     """
+    Decorator for altering field docstrings via assign filter functions.
+
     A decorator for assign filter functions that allows them to declaratively
     modify the docstring of the field they are used on.
 
@@ -581,7 +643,7 @@
 def read_only_assign_filter(
         instance: POD, field: Field, old: "Any", new: "Any") -> "Any":
     """
-    An assign filter that makes a field read-only
+    An assign filter that makes a field read-only.
 
     The field can be only assigned if the old value is ``UNSET``, that is,
     during the initial construction of a POD object.
@@ -639,7 +701,7 @@
 def type_check_assign_filter(
         instance: POD, field: Field, old: "Any", new: "Any") -> "Any":
     """
-    An assign filter that type-checks the value according to the field type
+    An assign filter that type-checks the value according to the field type.
 
     The field must have a valid python type object stored in the .type field.
 
@@ -666,7 +728,10 @@
 
 
 class sequence_type_check_assign_filter:
+
     """
+    Assign filter for typed sequences.
+
     An assign filter for typed sequences (lists or tuples) that must contain an
     object of the given type.
     """
@@ -689,7 +754,7 @@
             self, instance: POD, field: Field, old: "Any", new: "Any"
     ) -> "Any":
         """
-        An assign filter that type-checks the value of all sequence elements
+        An assign filter that type-checks the value of all sequence elements.
 
         :param instance:
             A subclass of :class:`POD` that contains ``field``