Fix broken dependency linearization.
authorW. Trevor King <wking@tremily.us>
Sat, 20 Oct 2012 02:20:20 +0000 (22:20 -0400)
committerW. Trevor King <wking@tremily.us>
Sat, 20 Oct 2012 02:20:20 +0000 (22:20 -0400)
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.

doc/module_dependencies.txt
pycomedi/channel.pyx
pycomedi/device.pxd
pycomedi/device.pyx
pycomedi/device_holder.pxd [new file with mode: 0644]
pycomedi/device_holder.pyx [new file with mode: 0644]
pycomedi/subdevice.pxd
pycomedi/subdevice.pyx
pycomedi/subdevice_holder.pxd [new file with mode: 0644]
pycomedi/subdevice_holder.pyx [new file with mode: 0644]

index 92b4f8d4b68a49e8a6a769b35abdfd47b5d10b62..ec344e9de6242630962ad18fa0a2e039d2ede4dc 100644 (file)
@@ -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
index 825744e277098cc9dfa5e8a585bc2bf5bcede732..ef204c057afbeccfac5a4da383ded24bc5aaabf3 100644 (file)
@@ -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(
index 22152d9cd0b4fa6100745ce886556d253a01686f..9897881f9502eddddf9cd206ddc36ab378b10196 100644 (file)
 "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
 
index 4f23d79185b34251dd13f05034eb3e0490f75966..b0360eb571bdedb184bae8b1112e7356f351033d 100644 (file)
@@ -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 (file)
index 0000000..9b49a57
--- /dev/null
@@ -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 (file)
index 0000000..5acfacd
--- /dev/null
@@ -0,0 +1,7 @@
+# Copyright
+
+
+cdef class DeviceHolder (object):
+    "Minimal comedi_t * wrapper to avoid circular imports"
+    def __cinit__(self):
+        self.device = NULL
index 31ef7725e50526ca762d9c8b4644971fcfc80344..1b85d28face06d168ba32b27ca6857441719194d 100644 (file)
 
 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
index 425c5b97b3fa35c4cc90e836ea34bc007d1702a4..6acb33a6697c9e2f42cbd7a53b28811aa0b7c5fb 100644 (file)
@@ -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 (file)
index 0000000..daa982c
--- /dev/null
@@ -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 (file)
index 0000000..981ce22
--- /dev/null
@@ -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