Merge remote-tracking branch 'origin/main' into main

Author: tomvanmele

CHANGELOG.md

@@ -11,15 +11,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+* Changed new artist registration to check if subclass.
+
 ### Removed
 
 
 ## [1.9.1] 2021-10-22
 
 ### Added
 
-* Added `Plane.offset`
-* Added `is_mesh_closed` property to `compas.datastructures.mesh_slice_plane`
+* Added `Plane.offset`.
+* Added `is_mesh_closed` property to `compas.datastructures.mesh_slice_plane`.
 
 ### Changed
 

src/compas/artists/artist.py

@@ -2,7 +2,10 @@
 from __future__ import division
 from __future__ import print_function
 
+import inspect
 from abc import abstractmethod
+
+from compas.artists import DataArtistNotRegistered
 from compas.plugins import pluggable
 
 
@@ -55,6 +58,21 @@ def build_as(item, artist_type, **kwargs):
         artist = artist_type(item, **kwargs)
         return artist
 
+    @staticmethod
+    def get_artist_cls(data, **kwargs):
+        dtype = type(data)
+        cls = None
+        if 'artist_type' in kwargs:
+            cls = kwargs['artist_type']
+        else:
+            for type_ in inspect.getmro(dtype):
+                cls = Artist.ITEM_ARTIST.get(type_)
+                if cls is not None:
+                    break
+        if cls is None:
+            raise DataArtistNotRegistered('No artist is registered for this data type: {}'.format(dtype))
+        return cls
+
     @staticmethod
     def clear():
         return clear()

src/compas/plugins.py

@@ -45,6 +45,7 @@
     :toctree: generated/
     :nosignatures:
 
+    IncompletePluginImplError
     PluginNotInstalledError
 """
 from __future__ import absolute_import
@@ -60,8 +61,10 @@
     'pluggable',
     'plugin',
     'plugin_manager',
+    'IncompletePluginImplError',
     'PluginManager',
     'PluginNotInstalledError',
+    'PluginValidator',
 ]
 
 
@@ -81,6 +84,11 @@ def _get_extension_point_url_from_method(domain, category, plugin_method):
     return '{}/{}/{}'.format(domain, category, name).replace('//', '/')
 
 
+class IncompletePluginImplError(Exception):
+    """Exception raised when a plugin does not have implementations for all abstract methods of its base class."""
+    pass
+
+
 class PluginImpl(object):
     """Internal data class to keep track of a loaded plugin implementation.
 
@@ -392,7 +400,11 @@ def try_import(self, module_name):
         try:
             module = __import__(module_name, fromlist=['__name__'], level=0)
             self._cache[module_name] = True
-        except ImportError:
+
+        # There are two types of possible failure modes:
+        # 1) cannot be imported, or
+        # 2) is a python 3 module and we're in IPY, which causes a SyntaxError
+        except (ImportError, SyntaxError):
             self._cache[module_name] = False
 
         return module
@@ -416,46 +428,56 @@ def check_importable(self, module_name):
         return self._cache[module_name]
 
 
-def verify_requirement(manager, requirement):
-    if callable(requirement):
-        return requirement()
+class PluginValidator(object):
+    """Plugin Validator handles validation of plugins."""
 
-    return manager.importer.check_importable(requirement)
+    def __init__(self, manager):
+        self.manager = manager
 
+    def verify_requirement(self, requirement):
+        if callable(requirement):
+            return requirement()
 
-def is_plugin_selectable(plugin, manager):
-    if plugin.opts['requires']:
-        importable_requirements = (verify_requirement(manager, requirement) for requirement in plugin.opts['requires'])
+        return self.manager.importer.check_importable(requirement)
 
-        if not all(importable_requirements):
-            if manager.DEBUG:
-                print('Requirements not satisfied. Plugin will not be used: {}'.format(plugin.id))
-            return False
+    def is_plugin_selectable(self, plugin):
+        if plugin.opts['requires']:
+            importable_requirements = (self.verify_requirement(requirement) for requirement in plugin.opts['requires'])
 
-    return True
+            if not all(importable_requirements):
+                if self.manager.DEBUG:
+                    print('Requirements not satisfied. Plugin will not be used: {}'.format(plugin.id))
+                return False
 
+        return True
 
-def select_plugin(extension_point_url, manager):
-    if manager.DEBUG:
-        print('Extension Point URL {} invoked. Will select a matching plugin'.format(extension_point_url))
+    def select_plugin(self, extension_point_url):
+        if self.manager.DEBUG:
+            print('Extension Point URL {} invoked. Will select a matching plugin'.format(extension_point_url))
 
-    plugins = manager.registry.get(extension_point_url) or []
-    for plugin in plugins:
-        if is_plugin_selectable(plugin, manager):
-            return plugin
+        plugins = self.manager.registry.get(extension_point_url) or []
+        for plugin in plugins:
+            if self.is_plugin_selectable(plugin):
+                return plugin
 
-    # Nothing found, raise
-    raise PluginNotInstalledError('Plugin not found for extension point URL: {}'.format(extension_point_url))
+        # Nothing found, raise
+        raise PluginNotInstalledError('Plugin not found for extension point URL: {}'.format(extension_point_url))
 
+    def collect_plugins(self, extension_point_url):
+        if self.manager.DEBUG:
+            print('Extension Point URL {} invoked. Will select a matching plugin'.format(extension_point_url))
 
-def collect_plugins(extension_point_url, manager):
-    if manager.DEBUG:
-        print('Extension Point URL {} invoked. Will select a matching plugin'.format(extension_point_url))
+        plugins = self.manager.registry.get(extension_point_url) or []
+        return [plugin for plugin in plugins if self.is_plugin_selectable(plugin)]
 
-    plugins = manager.registry.get(extension_point_url) or []
-    return [plugin for plugin in plugins if is_plugin_selectable(plugin, manager)]
+    @staticmethod
+    def ensure_implementations(cls):
+        for name, value in inspect.getmembers(cls):
+            if inspect.isfunction(value) or inspect.ismethod(value):
+                if hasattr(value, '__isabstractmethod__'):
+                    raise IncompletePluginImplError('Abstract method not implemented: {}'.format(value))
 
 
 plugin_manager = PluginManager()
-_select_plugin = functools.partial(select_plugin, manager=plugin_manager)
-_collect_plugins = functools.partial(collect_plugins, manager=plugin_manager)
+_select_plugin = PluginValidator(plugin_manager).select_plugin
+_collect_plugins = PluginValidator(plugin_manager).collect_plugins

src/compas_blender/artists/__init__.py

@@ -62,13 +62,12 @@
     BlenderArtist
 
 """
-import inspect
 
 import compas_blender
 
 from compas.plugins import plugin
+from compas.plugins import PluginValidator
 from compas.artists import Artist
-from compas.artists import DataArtistNotRegistered
 
 from compas.geometry import Box
 from compas.geometry import Capsule
@@ -130,20 +129,9 @@ def new_artist_blender(cls, *args, **kwargs):
 
     data = args[0]
 
-    if 'artist_type' in kwargs:
-        cls = kwargs['artist_type']
-    else:
-        dtype = type(data)
-        if dtype not in BlenderArtist.ITEM_ARTIST:
-            raise DataArtistNotRegistered('No Blender artist is registered for this data type: {}'.format(dtype))
-        cls = BlenderArtist.ITEM_ARTIST[dtype]
+    cls = Artist.get_artist_cls(data, **kwargs)
 
-    # TODO: move this to the plugin module and/or to a dedicated function
-
-    for name, value in inspect.getmembers(cls):
-        if inspect.isfunction(value):
-            if hasattr(value, '__isabstractmethod__'):
-                raise Exception('Abstract method not implemented: {}'.format(value))
+    PluginValidator.ensure_implementations(cls)
 
     return super(Artist, cls).__new__(cls)
 

src/compas_ghpython/artists/__init__.py

@@ -63,12 +63,10 @@
 """
 from __future__ import absolute_import
 
-import inspect
-
 from compas.plugins import plugin
+from compas.plugins import PluginValidator
 from compas.artists import Artist
 from compas.artists import ShapeArtist
-from compas.artists import DataArtistNotRegistered
 
 from compas.geometry import Box
 from compas.geometry import Capsule
@@ -170,20 +168,9 @@ def new_artist_gh(cls, *args, **kwargs):
 
     data = args[0]
 
-    if 'artist_type' in kwargs:
-        cls = kwargs['artist_type']
-    else:
-        dtype = type(data)
-        if dtype not in GHArtist.ITEM_ARTIST:
-            raise DataArtistNotRegistered('No GH artist is registered for this data type: {}'.format(dtype))
-        cls = GHArtist.ITEM_ARTIST[dtype]
-
-    # TODO: move this to the plugin module and/or to a dedicated function
+    cls = Artist.get_artist_cls(data, **kwargs)
 
-    for name, value in inspect.getmembers(cls):
-        if inspect.ismethod(value):
-            if hasattr(value, '__isabstractmethod__'):
-                raise Exception('Abstract method not implemented: {}'.format(value))
+    PluginValidator.ensure_implementations(cls)
 
     return super(Artist, cls).__new__(cls)
 
@@ -204,7 +191,7 @@ def new_artist_gh(cls, *args, **kwargs):
     'PolygonArtist',
     'PolyhedronArtist',
     'PolylineArtist',
-    'RobotModelArtist'
+    'RobotModelArtist',
     'SphereArtist',
     'TorusArtist',
     'VectorArtist',

src/compas_plotters/artists/__init__.py

@@ -42,11 +42,10 @@
     PlotterArtist
 
 """
-import inspect
 
 from compas.plugins import plugin
+from compas.plugins import PluginValidator
 from compas.artists import Artist
-from compas.artists import DataArtistNotRegistered
 
 from compas.geometry import Point
 from compas.geometry import Vector
@@ -102,20 +101,9 @@ def new_artist_plotter(cls, *args, **kwargs):
 
     data = args[0]
 
-    if 'artist_type' in kwargs:
-        cls = kwargs['artist_type']
-    else:
-        dtype = type(data)
-        if dtype not in PlotterArtist.ITEM_ARTIST:
-            raise DataArtistNotRegistered('No Plotter artist is registered for this data type: {}'.format(dtype))
-        cls = PlotterArtist.ITEM_ARTIST[dtype]
-
-    # TODO: move this to the plugin module and/or to a dedicated function
+    cls = Artist.get_artist_cls(data, **kwargs)
 
-    for name, value in inspect.getmembers(cls):
-        if inspect.isfunction(value):
-            if hasattr(value, '__isabstractmethod__'):
-                raise Exception('Abstract method not implemented: {}'.format(value))
+    PluginValidator.ensure_implementations(cls)
 
     return super(Artist, cls).__new__(cls)
 
@@ -130,5 +118,5 @@ def new_artist_plotter(cls, *args, **kwargs):
     'CircleArtist',
     'EllipseArtist',
     'MeshArtist',
-    'NetworkArtist'
+    'NetworkArtist',
 ]

src/compas_rhino/artists/__init__.py

@@ -73,12 +73,10 @@
 """
 from __future__ import absolute_import
 
-import inspect
-
 from compas.plugins import plugin
+from compas.plugins import PluginValidator
 from compas.artists import Artist
 from compas.artists import ShapeArtist
-from compas.artists import DataArtistNotRegistered
 
 from compas.geometry import Circle
 from compas.geometry import Frame
@@ -196,20 +194,9 @@ def new_artist_rhino(cls, *args, **kwargs):
 
     data = args[0]
 
-    if 'artist_type' in kwargs:
-        cls = kwargs['artist_type']
-    else:
-        dtype = type(data)
-        if dtype not in RhinoArtist.ITEM_ARTIST:
-            raise DataArtistNotRegistered('No Rhino artist is registered for this data type: {}'.format(dtype))
-        cls = RhinoArtist.ITEM_ARTIST[dtype]
-
-    # TODO: move this to the plugin module and/or to a dedicated function
+    cls = Artist.get_artist_cls(data, **kwargs)
 
-    for name, value in inspect.getmembers(cls):
-        if inspect.ismethod(value):
-            if hasattr(value, '__isabstractmethod__'):
-                raise Exception('Abstract method not implemented: {}'.format(value))
+    PluginValidator.ensure_implementations(cls)
 
     return super(Artist, cls).__new__(cls)
 
@@ -235,5 +222,5 @@ def new_artist_rhino(cls, *args, **kwargs):
     'MeshArtist',
     'NetworkArtist',
     'VolMeshArtist',
-    'RobotModelArtist'
+    'RobotModelArtist',
 ]

tests/compas/artists/test_artists.py

@@ -0,0 +1,43 @@
+from compas.artists import Artist
+
+
+class FakeArtist(Artist):
+    def draw(self):
+        pass
+
+
+class FakeSubArtist(Artist):
+    def draw(self):
+        pass
+
+
+class FakeItem(object):
+    pass
+
+
+class FakeSubItem(FakeItem):
+    pass
+
+
+def test_get_artist_cls_with_orderly_registration():
+    Artist.register(FakeItem, FakeArtist)
+    Artist.register(FakeSubItem, FakeSubArtist)
+    item = FakeItem()
+    artist = Artist.get_artist_cls(item)
+    assert artist == FakeArtist
+
+    item = FakeSubItem()
+    artist = Artist.get_artist_cls(item)
+    assert artist == FakeSubArtist
+
+
+def test_get_artist_cls_with_out_of_order_registration():
+    Artist.register(FakeSubItem, FakeSubArtist)
+    Artist.register(FakeItem, FakeArtist)
+    item = FakeItem()
+    artist = Artist.get_artist_cls(item)
+    assert artist == FakeArtist
+
+    item = FakeSubItem()
+    artist = Artist.get_artist_cls(item)
+    assert artist == FakeSubArtist