xml_reflection: First step to enable deprecation of public access

CHANGELOG.rst

@@ -2,6 +2,8 @@
 Changelog for package urdfdom_py
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+* Deprecate internal details of `xml_reflection` classes.
+
 0.4.0 (2018-02-21)
 ------------------
 * Add Link.visual and Link.collision properties (`#28 <https://github.com/ros/urdf_parser_py/issues/28>`_)

README.md

@@ -1,45 +1,37 @@
 # urdf_parser_py
 
-## Development Setup
+## Authors
 
-You must manually run `setup.py`. For catkin development, you can install to $ws/../build/lib/pythonX.Y/dist-packages via
+*   Thomas Moulard - `urdfpy` implementation, integration
+*   David Lu - `urdf_python` implementation, integration
+*   Kelsey Hawkins - `urdf_parser_python` implementation, integration
+*   Antonio El Khoury - bugfixes
+*   Eric Cousineau - reflection
+*   Ioan Sucan
+*   Jackie Kay
+*   Chris LaLancette (maintainer)
+*   Shane Loretz (maintainer)
 
-	devel_prefix=$(cd $(catkin_find --first-only)/.. && pwd)
-	cd ../urdf_parser_py
-	python setup.py install --install-layout deb --prefix $devel_prefix
+## Features
 
-## Authors
+*   URDF
+*   SDF (very basic coverage)
+*   XML Saving / Loading
+    * Some attempts to preserve original ordering; comments are stripped out,
+    however.
+
+## Todo
+
+1.  Deprecate public access to `xml_reflection`.
+2.  Make a direct, two-way URDF <-> SDF converter when kinematics are not an
+    issue.
+
+## Development Setup
+
+For ease of developing, you may manually run `setup.py`.
+For catkin development, you can install to
+`${ws}/../build/lib/pythonX.Y/dist-packages` via:
 
-*	Thomas Moulard - `urdfpy` implementation, integration
-*	David Lu - `urdf_python` implementation, integration
-*	Kelsey Hawkins - `urdf_parser_python` implementation, integration
-*	Antonio El Khoury - bugfixes
-*	Eric Cousineau - reflection update
-
-## Reflection
-
-This an attempt to generalize the structure of the URDF via reflection to make it easier to extend. This concept is taken from Gazebo's SDF structure, and was done with SDF in mind to a) make an SDF parser and b) make a simple converter between URDF and SDF.
-
-### Changes
-
-*	Features:
-	*	Transmission and basic Gazebo nodes.
-	*	General aggregate types, preserving order
-	*	Dumping to YAML, used for printing to string (dictionaries do not preserve attribute ordering)
-*	XML Parsing: minidom has been swapped out with lxml.etree, but it should not be hard to change that back. Maybe Sax could be used for event-driven parsing.
-*	API:
-	*	Loading methods rely primarily on instance methods rather than static methods, mirroring Gazebo's SDF construct-then-load method
-	*	Renamed static `parse_xml()` to `from_xml()`, and renamed `load_*` methods to `from_*` if they are static
-
-### Todo
-
-1.	Support additional formats (SDF, drakeURDF, etc.)
-	*	Parse Gazebo's SDF definition files at some point? For speed's sake, parse it and have it generate code to use?
-	*	Consider auto-generating modules from schemas such as [urdf.xsd](https://github.com/ros/urdfdom/blob/master/xsd/urdf.xsd). This can extend to [SDF](http://sdformat.org/schemas/model.xsd), [drakeURDF](https://github.com/RobotLocomotion/drake/blob/master/drake/doc/drakeURDF.xsd).
-2.	Make a direct, two-way URDF <-> SDF converter.
-	*	Gazebo has the ability to load URDFs and save SDFs, but it lumps everything together
-3.	Consider a cleaner implementation for reflection.
-	*	Make the names a little clearer, especially the fact that `from_xml` and `to_xml` write to a node, but do not create a new one.
-	*	Abstraction layer is not clear. Should explicitly use abstract classes, and try to really clarify the dispatch order (`xmlr.Element`, `xmlr.Param`, `xmlr.Object`, etc.)
-4.	Figure out good policy for handling default methods. If saving to XML, write out default values, or leave them out for brevity (and to leave it open for change)? Might be best to add that as an option.
-5.	Find a lightweight package that can handle the reflection aspect more elegantly. Enthought traits? IPython's spinoff of traits?
+    devel_prefix=$(cd $(catkin_find --first-only)/.. && pwd)
+    cd ../urdf_parser_py
+    python setup.py install --install-layout deb --prefix ${devel_prefix}

package.xml

@@ -10,7 +10,7 @@
   <author>David Lu</author>
   <author>Kelsey Hawkins</author>
   <author>Antonio El Khoury</author>
-  <author>Eric Cousineau</author>
+  <author email="[email protected]">Eric Cousineau</author>
   <author email="[email protected]">Ioan Sucan</author>
   <author email="[email protected]">Jackie Kay</author>
 

src/urdf_parser_py/__init__.py

@@ -0,0 +1,68 @@
+"""
+Python implementation of the URDF parser.
+"""
+
+import functools
+import warnings
+
+
+class _DeprecatedDescriptor(object):
+    def __init__(self, attr):
+        self._attr = attr
+
+    def _warn(self):
+        raise NotImplemented
+
+    def __get__(self, obj, objtype):
+        self._warn()
+        if obj is None:
+            return getattr(objtype, self._attr)
+        else:
+            return getattr(obj, self._attr)
+
+    def __set__(self, obj, value):
+        self._warn()
+        setattr(obj, self._attr, value)
+
+    def __del__(self, obj):
+        self._warn()
+        delattr(obj, self._attr)
+
+
+class _NowPrivateDescriptor(_DeprecatedDescriptor):
+    # Implements the descriptor interface to warn about deprecated access.
+    def __init__(self, private):
+        _DeprecatedDescriptor.__init__(self, private)
+        self._private = private
+        self._old_public = self._private.lstrip('_')
+        self.__doc__ = "Deprecated propery '{}'".format(self._old_public)
+
+    def _warn(self):
+        warnings.warn(
+            "'{}' is deprecated, and will be removed in future releases."
+                .format(self._old_public),
+            category=DeprecationWarning, stacklevel=1)
+
+
+def _now_private_property(private):
+    # Indicates that a property (or method) is now private.
+    return _NowPrivateDescriptor(private)
+
+
+class _RenamedDescriptor(_DeprecatedDescriptor):
+    # Implements the descriptor interface to warn about deprecated access.
+    def __init__(self, old, new):
+        _DeprecatedDescriptor.__init__(self, new)
+        self._old = old
+        self._new = new
+        self.__doc__ = "Deprecated propery '{}'".format(self._old)
+
+    def _warn(self):
+        warnings.warn(
+            "'{}' is deprecated, please use '{}' instead.".format(
+                self._old, self._new),
+            category=DeprecationWarning, stacklevel=1)
+
+
+def _renamed_property(old, new):
+    return _RenamedDescriptor(old, new)

src/urdf_parser_py/_xml_reflection.py

@@ -0,0 +1,5 @@
+# TODO(eacousineau): Move all symbols from `.xml_reflection` into here.
+from urdf_parser_py.xml_reflection.basics import *
+# Import full module so that tests can easily monkey patch `on_error`.
+from urdf_parser_py.xml_reflection import core
+from urdf_parser_py.xml_reflection.core import *

src/urdf_parser_py/sdf.py

@@ -1,12 +1,10 @@
-from urdf_parser_py.xml_reflection.basics import *
-import urdf_parser_py.xml_reflection as xmlr
+from urdf_parser_py import _now_private_property
+import urdf_parser_py._xml_reflection as _xmlr
 
-# What is the scope of plugins? Model, World, Sensor?
+_xmlr.start_namespace('sdf')
 
-xmlr.start_namespace('sdf')
 
-
-class Pose(xmlr.Object):
+class Pose(_xmlr.Object):
     def __init__(self, vec=None, extra=None):
         self.xyz = None
         self.rpy = None
@@ -32,37 +30,37 @@ def as_vec(self):
         rpy = self.rpy if self.rpy else [0, 0, 0]
         return xyz + rpy
 
-    def read_xml(self, node):
+    def _read_xml(self, node):
         # Better way to do this? Define type?
-        vec = get_type('vector6').read_xml(node)
-        self.load_vec(vec)
+        vec = _xmlr.get_type('vector6').read_xml_value(node)
+        self.from_vec(vec)
 
-    def write_xml(self, node):
+    def _write_xml(self, node):
         vec = self.as_vec()
-        get_type('vector6').write_xml(node, vec)
+        _xmlr.get_type('vector6').write_xml_value(node, vec)
 
-    def check_valid(self):
+    def _check_valid(self):
         assert self.xyz is not None or self.rpy is not None
 
 
-name_attribute = xmlr.Attribute('name', str)
-pose_element = xmlr.Element('pose', Pose, False)
+_name_attribute = _xmlr.Attribute('name', str)
+_pose_element = _xmlr.Element('pose', Pose, required=False)
 
 
-class Entity(xmlr.Object):
+class Entity(_xmlr.Object):
     def __init__(self, name=None, pose=None):
         self.name = name
         self.pose = pose
 
 
-xmlr.reflect(Entity, params=[
-    name_attribute,
-    pose_element
+_xmlr.reflect(Entity, params=[
+    _name_attribute,
+    _pose_element
 ])
 
 
-class Inertia(xmlr.Object):
-    KEYS = ['ixx', 'ixy', 'ixz', 'iyy', 'iyz', 'izz']
+class Inertia(_xmlr.Object):
+    _KEYS = ['ixx', 'ixy', 'ixz', 'iyy', 'iyz', 'izz']
 
     def __init__(self, ixx=0.0, ixy=0.0, ixz=0.0, iyy=0.0, iyz=0.0, izz=0.0):
         self.ixx = ixx
@@ -79,24 +77,21 @@ def to_matrix(self):
             [self.ixz, self.iyz, self.izz]]
 
 
-xmlr.reflect(Inertia,
-             params=[xmlr.Element(key, float) for key in Inertia.KEYS])
-
-# Pretty much copy-paste... Better method?
-# Use multiple inheritance to separate the objects out so they are unique?
+_xmlr.reflect(Inertia, tag='inertia',
+             params=[_xmlr.Element(key, float) for key in Inertia._KEYS])
 
 
-class Inertial(xmlr.Object):
+class Inertial(_xmlr.Object):
     def __init__(self, mass=0.0, inertia=None, pose=None):
         self.mass = mass
         self.inertia = inertia
         self.pose = pose
 
 
-xmlr.reflect(Inertial, params=[
-    xmlr.Element('mass', float),
-    xmlr.Element('inertia', Inertia),
-    pose_element
+_xmlr.reflect(Inertial, tag='inertial', params=[
+    _xmlr.Element('mass', float),
+    _xmlr.Element('inertia', Inertia),
+    _pose_element
 ])
 
 
@@ -107,14 +102,21 @@ def __init__(self, name=None, pose=None, inertial=None, kinematic=False):
         self.kinematic = kinematic
 
 
-xmlr.reflect(Link, parent_cls=Entity, params=[
-    xmlr.Element('inertial', Inertial),
-    xmlr.Attribute('kinematic', bool, False),
-    xmlr.AggregateElement('visual', Visual, var='visuals'),
-    xmlr.AggregateElement('collision', Collision, var='collisions')
+_xmlr.reflect(Link, tag='link', parent_cls=Entity, params=[
+    _xmlr.Element('inertial', Inertial),
+    _xmlr.Attribute('kinematic', bool, False),
+    _xmlr.AggregateElement('visual', Visual, var='visuals'),
+    _xmlr.AggregateElement('collision', Collision, var='collisions')
 ])
 
 
+class Joint(Entity):
+    pass
+
+
+_xmlr.reflect(Joint, tag='joint', parent_cls=Entity, params=[])
+
+
 class Model(Entity):
     def __init__(self, name=None, pose=None):
         Entity.__init__(self, name, pose)
@@ -123,10 +125,10 @@ def __init__(self, name=None, pose=None):
         self.plugins = []
 
 
-xmlr.reflect(Model, parent_cls=Entity, params=[
-    xmlr.AggregateElement('link', Link, var='links'),
-    xmlr.AggregateElement('joint', Joint, var='joints'),
-    xmlr.AggregateElement('plugin', Plugin, var='plugins')
+_xmlr.reflect(Model, parent_cls=Entity, params=[
+    _xmlr.AggregateElement('link', Link, var='links'),
+    _xmlr.AggregateElement('joint', Joint, var='joints'),
+    _xmlr.AggregateElement('plugin', Plugin, var='plugins')
 ])
 
-xmlr.end_namespace('sdf')
+_xmlr.end_namespace('sdf')