From: W. Trevor King <wking@tremily.us>
Date: Sat, 20 Oct 2012 02:20:20 +0000 (-0400)
Subject: Fix broken dependency linearization.
X-Git-Tag: 0.6~4
X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=165a45dca9dd31ee9b31f1b1c7fce15217feb451;p=pycomedi.git

Fix broken dependency linearization.

I had tried to break circular dependencies with:

  commit c19dd6ff6ce26beaf1b0ffc57d505b71898f8d0f
  Author: W. Trevor King <wking@tremily.us>
  Date:   Fri Oct 19 08:58:34 2012 -0400

    Break circular dependencies for Python 3 compatibility.

Which was causing errors like:

  Exception AttributeError:
    "'pycomedi.device.Device' object has no attribute 'device'"
    in 'pycomedi.subdevice.Subdevice._device'

because when you treat Device instances as Python objects, you cannot
access their private (C-only) attributes.  This commit avoids that
problem by adding low level classes (DeviceHolder and SubdeviceHolder)
that can be imported by both the high level classes (Device and
Subdevice) and their users (Subdevice and Channel), avoiding circular
dependencies while preserving critical C-only attributes.
---

diff --git a/doc/module_dependencies.txt b/doc/module_dependencies.txt
index 92b4f8d..ec344e9 100644
--- a/doc/module_dependencies.txt
+++ b/doc/module_dependencies.txt
@@ -1,38 +1,20 @@
 With Python 3, Cython is pickier about circular dependencies.  This is
-all well and good, but sometimes lower level objects (e.g. Subdevice
-instances) need pointers from higher level objects (e.g. Device
-instances).  How does that work?
+all well and good, but sometimes lower level objects
+(e.g. ``Subdevice`` instances) need pointers from higher level objects
+(e.g. ``Device`` instances).  How does that work?
 
 All of the C types used in the library are defined in library-specific
 ``_*_h.pxd`` files at the bottom of the dependency tree.  The
 low-level objects don't really need the higher level objects (which
 would create a circular import), they need the *pointers* held by the
-higher level object.  The solution is to store higher-level objects as
-generic Python objects (e.g. Subdevice.device).  When you need the
-device ``comedi_t *`` for a subdevice method, use
-``Subdevice._device``, which assumes the ``Subdevice.device`` has been
-setup with a ``Device`` instance, and extracts the pointer with the
-appropriate casting.
-
-This deals with the circular imports, but is the casting safe?
-Kindof.  While it would be nice to do something like::
-
-  cdef class Subdevice (object):
-      cdef _comedilib_h.comedi_t * _device(self):
-          if not isinstance(self.device, _device.Device):
-              raise ValueError(self.device)
-          return <_comedilib_h.comedi_t *> self.device.device
-
-we can't perform the check without circularly importing
-``pycomedi.device``.  So the cast is blind, which could cause all
-sorts of problems.
-
-In practice, however, all ``Subdevice`` instances are likely to come
-from the ``Device.subdevice()`` method, which sets up the ``.device``
-attribute appropriately.  This means that while you *could* put
-something else in the ``.device`` attribute and blow things up, you
-probably won't do so accidentally.  This conforms to the Python style
-of assuming that we are all consenting adults.
+higher level object.  The solution is create minimal wrappers
+(e.g. ``DeviceHolder``), holding only the typed attributes
+(``DeviceHolder.device``) and basic methods.  These minimal classes
+can be subclassed by the full-fledged class (``Device``), but they can
+*also* be imported by the lower level modules and used to define
+attributes (``Subdevice.device``) that can then hold either the
+minimal (``DeviceHolder``) or the full-fledged (``Device``) version of
+the higher level object.
 
 For clarity in the above, I discussed the ``Device`` / ``Subdevice``
 relationship, but the same logic holds for similar high- / low-level
diff --git a/pycomedi/channel.pyx b/pycomedi/channel.pyx
index 825744e..ef204c0 100644
--- a/pycomedi/channel.pyx
+++ b/pycomedi/channel.pyx
@@ -27,6 +27,7 @@ cimport _comedilib_h
 from calibration cimport CalibratedConverter as _CalibratedConverter
 from calibration cimport Calibration as _Calibration
 from range cimport Range as _Range
+from subdevice_holder cimport SubdeviceHolder as _SubdeviceHolder
 
 from pycomedi import LOG as _LOG
 from chanspec import ChanSpec as _ChanSpec
@@ -57,7 +58,7 @@ cdef class Channel (object):
 
     >>> d.close()
     """
-    cdef public object subdevice  # pycomedi.subdevice.Subdevice
+    cdef public _SubdeviceHolder subdevice
     cdef public int index
 
     def __cinit__(self):
@@ -70,7 +71,7 @@ cdef class Channel (object):
         self.index = index
 
     cdef _comedilib_h.comedi_t * _device(self) except *:
-        return <_comedilib_h.comedi_t *> self.subdevice.device.device
+        return self.subdevice._device()
 
     def get_maxdata(self):
         ret = _comedilib_h.comedi_get_maxdata(
diff --git a/pycomedi/device.pxd b/pycomedi/device.pxd
index 22152d9..9897881 100644
--- a/pycomedi/device.pxd
+++ b/pycomedi/device.pxd
@@ -17,11 +17,11 @@
 "Expose `Device` internals at the C level for other Cython modules"
 
 cimport _comedilib_h
+from device_holder cimport DeviceHolder as _DeviceHolder
 from instruction cimport Insn as _Insn
 
 
-cdef class Device (object):
-    cdef _comedilib_h.comedi_t * device
+cdef class Device (_DeviceHolder):
     cdef public object file
     cdef public object filename
 
diff --git a/pycomedi/device.pyx b/pycomedi/device.pyx
index 4f23d79..b0360eb 100644
--- a/pycomedi/device.pyx
+++ b/pycomedi/device.pyx
@@ -27,12 +27,14 @@ cimport _comedi_h
 cimport _comedilib_h
 import _error
 from calibration import Calibration as _Calibration
+from device_holder cimport DeviceHolder as _DeviceHolder
+from device_holder import DeviceHolder as _DeviceHolder
 from instruction cimport Insn as _Insn
 from instruction import Insn as _Insn
 from subdevice import Subdevice as _Subdevice
 
 
-cdef class Device (object):
+cdef class Device (_DeviceHolder):
     """A Comedi device
 
     >>> from . import constant
@@ -99,7 +101,6 @@ cdef class Device (object):
     >>> d.close()
     """
     def __cinit__(self):
-        self.device = NULL
         self.file = None
         self.filename = None
 
diff --git a/pycomedi/device_holder.pxd b/pycomedi/device_holder.pxd
new file mode 100644
index 0000000..9b49a57
--- /dev/null
+++ b/pycomedi/device_holder.pxd
@@ -0,0 +1,7 @@
+# Copyright
+
+cimport _comedilib_h
+
+
+cdef class DeviceHolder (object):
+    cdef _comedilib_h.comedi_t * device
diff --git a/pycomedi/device_holder.pyx b/pycomedi/device_holder.pyx
new file mode 100644
index 0000000..5acfacd
--- /dev/null
+++ b/pycomedi/device_holder.pyx
@@ -0,0 +1,7 @@
+# Copyright
+
+
+cdef class DeviceHolder (object):
+    "Minimal comedi_t * wrapper to avoid circular imports"
+    def __cinit__(self):
+        self.device = NULL
diff --git a/pycomedi/subdevice.pxd b/pycomedi/subdevice.pxd
index 31ef772..1b85d28 100644
--- a/pycomedi/subdevice.pxd
+++ b/pycomedi/subdevice.pxd
@@ -18,15 +18,13 @@
 
 cimport _comedilib_h
 from command cimport Command as _Command
+from subdevice_holder cimport SubdeviceHolder as _SubdeviceHolder
 
 
-cdef class Subdevice (object):
-    cdef public object device  # pycomedi.device.Device
-    cdef public int index
-
-    cdef _comedilib_h.comedi_t * _device(self) except *
+cdef class Subdevice (_SubdeviceHolder):
     cpdef dio_bitfield(self, unsigned int bits=*, write_mask=*, base_channel=*)
 
+
 cdef class StreamingSubdevice (Subdevice):
     cdef public _Command cmd
     cdef public list _command_test_errors
diff --git a/pycomedi/subdevice.pyx b/pycomedi/subdevice.pyx
index 425c5b9..6acb33a 100644
--- a/pycomedi/subdevice.pyx
+++ b/pycomedi/subdevice.pyx
@@ -25,10 +25,12 @@ from channel import Channel as _Channel
 import chanspec as _chanspec
 import constant as _constant
 import command as _command
+from subdevice_holder cimport SubdeviceHolder as _SubdeviceHolder
+from subdevice_holder import SubdeviceHolder as _SubdeviceHolder
 from utility import _subdevice_dtype, _subdevice_typecode
 
 
-cdef class Subdevice (object):
+cdef class Subdevice (_SubdeviceHolder):
     """Class bundling subdevice-related functions
 
     >>> from .device import Device
@@ -65,18 +67,6 @@ cdef class Subdevice (object):
 
     >>> d.close()
     """
-    def __cinit__(self):
-        self.index = -1
-        self.device = None
-
-    def __init__(self, device, index):
-        super(Subdevice, self).__init__()
-        self.device = device
-        self.index = index
-
-    cdef _comedilib_h.comedi_t * _device(self) except *:
-        return <_comedilib_h.comedi_t *> self.device.device
-
     def get_type(self):
         "Type of subdevice (from `SUBDEVICE_TYPE`)"
         ret = _comedilib_h.comedi_get_subdevice_type(
diff --git a/pycomedi/subdevice_holder.pxd b/pycomedi/subdevice_holder.pxd
new file mode 100644
index 0000000..daa982c
--- /dev/null
+++ b/pycomedi/subdevice_holder.pxd
@@ -0,0 +1,11 @@
+# Copyright
+
+cimport _comedilib_h
+from device_holder cimport DeviceHolder as _DeviceHolder
+
+
+cdef class SubdeviceHolder (object):
+    cdef public _DeviceHolder device
+    cdef public int index
+
+    cdef _comedilib_h.comedi_t * _device(self) except *
diff --git a/pycomedi/subdevice_holder.pyx b/pycomedi/subdevice_holder.pyx
new file mode 100644
index 0000000..981ce22
--- /dev/null
+++ b/pycomedi/subdevice_holder.pyx
@@ -0,0 +1,18 @@
+# Copyright
+
+cimport _comedilib_h
+
+
+cdef class SubdeviceHolder (object):
+    "Minimal subdevice wrapper to avoid circular imports"
+    def __cinit__(self):
+        self.device = None
+        self.index = -1
+
+    def __init__(self, device, index):
+        super(SubdeviceHolder, self).__init__()
+        self.device = device
+        self.index = index
+
+    cdef _comedilib_h.comedi_t * _device(self) except *:
+        return self.device.device