Merge pull request #60 from force-h2020/move-parameters-to-mco

Moved parameters into MCO

doc/source/api/force_bdss.core_plugins.dummy.dummy_dakota.rst

@@ -43,6 +43,14 @@ force_bdss.core_plugins.dummy.dummy_dakota.dakota_optimizer module
     :undoc-members:
     :show-inheritance:
 
+force_bdss.core_plugins.dummy.dummy_dakota.parameters module
+------------------------------------------------------------
+
+.. automodule:: force_bdss.core_plugins.dummy.dummy_dakota.parameters
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
 
 Module contents
 ---------------

doc/source/api/force_bdss.mco.parameters.rst

@@ -27,22 +27,6 @@ force_bdss.mco.parameters.base_mco_parameter_factory module
     :undoc-members:
     :show-inheritance:
 
-force_bdss.mco.parameters.core_mco_parameters module
-----------------------------------------------------
-
-.. automodule:: force_bdss.mco.parameters.core_mco_parameters
-    :members:
-    :undoc-members:
-    :show-inheritance:
-
-force_bdss.mco.parameters.mco_parameter_factory_registry module
----------------------------------------------------------------
-
-.. automodule:: force_bdss.mco.parameters.mco_parameter_factory_registry
-    :members:
-    :undoc-members:
-    :show-inheritance:
-
 
 Module contents
 ---------------

doc/source/api/force_bdss.mco.parameters.tests.rst

@@ -20,22 +20,6 @@ force_bdss.mco.parameters.tests.test_base_mco_parameter_factory module
     :undoc-members:
     :show-inheritance:
 
-force_bdss.mco.parameters.tests.test_core_mco_parameters module
----------------------------------------------------------------
-
-.. automodule:: force_bdss.mco.parameters.tests.test_core_mco_parameters
-    :members:
-    :undoc-members:
-    :show-inheritance:
-
-force_bdss.mco.parameters.tests.test_parameter_factory_registry module
-----------------------------------------------------------------------
-
-.. automodule:: force_bdss.mco.parameters.tests.test_parameter_factory_registry
-    :members:
-    :undoc-members:
-    :show-inheritance:
-
 
 Module contents
 ---------------

force_bdss/base_core_driver.py

@@ -7,9 +7,6 @@ from .bundle_registry_plugin import (
 )
 from .io.workflow_reader import WorkflowReader
 from .workspecs.workflow import Workflow
-from .mco.parameters.mco_parameter_factory_registry import (
-    MCOParameterFactoryRegistry)
-from .mco.parameters.core_mco_parameters import all_core_factories
 
 
 class BaseCoreDriver(Plugin):
@@ -20,24 +17,13 @@ class BaseCoreDriver(Plugin):
     #: The registry of the bundles.
     bundle_registry = Instance(BundleRegistryPlugin)
 
-    #: The registry of the MCO parameters
-    parameter_factory_registry = Instance(MCOParameterFactoryRegistry)
-
     #: Deserialized content of the workflow file.
     workflow = Instance(Workflow)
 
     def _bundle_registry_default(self):
         return self.application.get_plugin(BUNDLE_REGISTRY_PLUGIN_ID)
 
-    def _parameter_factory_registry_default(self):
-        registry = MCOParameterFactoryRegistry()
-        for f in all_core_factories():
-            registry.register(f)
-
-        return registry
-
     def _workflow_default(self):
-        reader = WorkflowReader(self.bundle_registry,
-                                self.parameter_factory_registry)
+        reader = WorkflowReader(self.bundle_registry)
         with open(self.application.workflow_filepath) as f:
             return reader.read(f)

force_bdss/bundle_registry_plugin.py

@@ -55,13 +55,13 @@ class BundleRegistryPlugin(Plugin):
 
         Raises
         ------
-        ValueError: if the entry is not found.
+        KeyError: if the entry is not found.
         """
         for ds in self.data_source_bundles:
             if ds.id == id:
                 return ds
 
-        raise ValueError(
+        raise KeyError(
             "Requested data source {} but don't know how "
             "to find it.".format(id))
 
@@ -77,13 +77,13 @@ class BundleRegistryPlugin(Plugin):
 
         Raises
         ------
-        ValueError: if the entry is not found.
+        KeyError: if the entry is not found.
         """
         for kpic in self.kpi_calculator_bundles:
             if kpic.id == id:
                 return kpic
 
-        raise ValueError(
+        raise KeyError(
             "Requested kpi calculator {} but don't know how "
             "to find it.".format(id))
 
@@ -99,11 +99,40 @@ class BundleRegistryPlugin(Plugin):
 
         Raises
         ------
-        ValueError: if the entry is not found.
+        KeyError: if the entry is not found.
         """
         for mco in self.mco_bundles:
             if mco.id == id:
                 return mco
 
-        raise ValueError("Requested MCO {} but don't know how "
-                         "to find it.".format(id))
+        raise KeyError("Requested MCO {} but don't know how "
+                       "to find it.".format(id))
+
+    def mco_parameter_factory_by_id(self, mco_id, parameter_id):
+        """Retrieves the MCO parameter factory for a given MCO id and
+        parameter id.
+
+        Parameters
+        ----------
+        mco_id: str
+            The MCO identifier string
+        parameter_id: str
+            the parameter identifier string
+
+        Returns
+        -------
+        An instance of BaseMCOParameterFactory.
+
+        Raises
+        ------
+        KeyError:
+            if the entry is not found
+        """
+        mco_bundle = self.mco_bundle_by_id(mco_id)
+
+        for factory in mco_bundle.parameter_factories():
+            if factory.id == parameter_id:
+                return factory
+
+        raise KeyError("Requested MCO parameter {}:{} but don't know"
+                       " how to find it.".format(mco_id, parameter_id))

force_bdss/core_plugins/dummy/dummy_dakota/dakota_bundle.py

 from traits.api import String
 from force_bdss.api import bundle_id, BaseMCOBundle
+from force_bdss.core_plugins.dummy.dummy_dakota.parameters import \
+    RangedMCOParameterFactory
 
 from .dakota_communicator import DummyDakotaCommunicator
 from .dakota_model import DummyDakotaModel
@@ -21,3 +23,8 @@ class DummyDakotaBundle(BaseMCOBundle):
 
     def create_communicator(self):
         return DummyDakotaCommunicator(self)
+
+    def parameter_factories(self):
+        return [
+            RangedMCOParameterFactory(self)
+        ]

force_bdss/mco/parameters/core_mco_parameters.py → force_bdss/core_plugins/dummy/dummy_dakota/parameters.py

 from traits.api import Float
 
-from ...ids import mco_parameter_id
-from .base_mco_parameter import BaseMCOParameter
+from force_bdss.ids import mco_parameter_id
+from force_bdss.mco.parameters.base_mco_parameter import BaseMCOParameter
 from force_bdss.mco.parameters.base_mco_parameter_factory import \
     BaseMCOParameterFactory
 
@@ -16,18 +16,7 @@ class RangedMCOParameter(BaseMCOParameter):
 
 class RangedMCOParameterFactory(BaseMCOParameterFactory):
     """The factory of the above model"""
-    id = mco_parameter_id("enthought", "ranged")
+    id = mco_parameter_id("enthought", "dummy_dakota", "ranged")
     model_class = RangedMCOParameter
     name = "Range"
     description = "A ranged parameter in floating point values."
-
-
-def all_core_factories():
-    """Produces a list of all factories contained in this module."""
-    import inspect
-
-    return [c()
-            for c in inspect.getmodule(all_core_factories).__dict__.values()
-            if inspect.isclass(c) and
-            c is not BaseMCOParameterFactory and
-            issubclass(c, BaseMCOParameterFactory)]

force_bdss/core_plugins/dummy/dummy_dakota/tests/test_dakota_bundle.py

@@ -41,3 +41,7 @@ class TestDakotaBundle(unittest.TestCase):
         bundle = DummyDakotaBundle(self.plugin)
         ds = bundle.create_optimizer()
         self.assertIsInstance(ds, DummyDakotaOptimizer)
+
+    def test_parameter_factories(self):
+        bundle = DummyDakotaBundle(self.plugin)
+        self.assertNotEqual(len(bundle.parameter_factories()), 0)

force_bdss/core_plugins/dummy/dummy_dakota/tests/test_dakota_communicator.py

@@ -13,7 +13,8 @@ from force_bdss.data_sources.data_source_parameters import DataSourceParameters
 
 from force_bdss.mco.parameters.base_mco_parameter_factory import \
     BaseMCOParameterFactory
-from force_bdss.mco.parameters.core_mco_parameters import RangedMCOParameter
+from force_bdss.core_plugins.dummy.dummy_dakota.parameters import \
+    RangedMCOParameter
 
 
 class TestDakotaCommunicator(unittest.TestCase):

force_bdss/core_plugins/dummy/dummy_dakota/tests/test_dakota_optimizer.py

 import unittest
 
-from force_bdss.core_plugins.dummy.dummy_dakota.dakota_model import \
+from force_bdss.core_plugins.dummy.dummy_dakota.parameters import (
+    RangedMCOParameter,
+    RangedMCOParameterFactory
+)
+from force_bdss.core_plugins.dummy.dummy_dakota.dakota_model import (
     DummyDakotaModel
+)
 from force_bdss.mco.base_mco_bundle import BaseMCOBundle
-from force_bdss.mco.parameters.core_mco_parameters import RangedMCOParameter, \
-    RangedMCOParameterFactory
 
 try:
     import mock

force_bdss/ids.py

@@ -29,23 +29,27 @@ def bundle_id(producer, identifier):
     -------
     str: an identifier to be used in the bundle.
     """
-    return _string_id("bundle", producer, identifier)
+    return _string_id(producer, "bundle", identifier)
 
 
-def mco_parameter_id(producer, identifier):
+def mco_parameter_id(producer, mco_identifier, parameter_identifier):
     """Creates an ID for an MCO parameter, so that it can be identified
     uniquely."""
-    return _string_id("mco_parameter", producer, identifier)
+    return _string_id(producer,
+                      "bundle",
+                      mco_identifier,
+                      "parameter",
+                      parameter_identifier)
 
 
 def plugin_id(producer, identifier):
     """Creates an ID for the plugins. These must be defined, otherwise
     the envisage system will complain (but not break)
     """
-    return _string_id("plugin", producer, identifier)
+    return _string_id(producer, "plugin", identifier)
 
 
-def _string_id(entity_namespace, producer, identifier):
+def _string_id(*args):
     """Creates an id for a generic entity.
 
     Parameters
@@ -68,7 +72,8 @@ def _string_id(entity_namespace, producer, identifier):
             " " not in entry and
             len(entry) != 0)
 
-    if not all(map(is_valid, [entity_namespace, producer, identifier])):
-        raise ValueError("Invalid parameters specified.")
+    if not all(map(is_valid, args)):
+        raise ValueError("One or more of the specified parameters was "
+                         "invalid: {}".format(str(args)))
 
-    return "force.bdss.{}.{}.{}".format(entity_namespace, producer, identifier)
+    return ".".join(["force", "bdss"]+list(args))

force_bdss/io/tests/test_workflow_reader.py

@@ -6,8 +6,6 @@ from force_bdss.bundle_registry_plugin import BundleRegistryPlugin
 from force_bdss.io.workflow_reader import (
     WorkflowReader,
     InvalidVersionException, InvalidFileException)
-from force_bdss.mco.parameters.mco_parameter_factory_registry import \
-    MCOParameterFactoryRegistry
 
 try:
     import mock
@@ -18,12 +16,8 @@ except ImportError:
 class TestWorkflowReader(unittest.TestCase):
     def setUp(self):
         self.mock_bundle_registry = mock.Mock(spec=BundleRegistryPlugin)
-        self.mock_mco_parameter_registry = mock.Mock(
-            spec=MCOParameterFactoryRegistry)
 
-        self.wfreader = WorkflowReader(
-            self.mock_bundle_registry,
-            self.mock_mco_parameter_registry)
+        self.wfreader = WorkflowReader(self.mock_bundle_registry)
 
     def test_initialization(self):
         self.assertEqual(self.wfreader.bundle_registry,

force_bdss/io/tests/test_workflow_writer.py

@@ -7,8 +7,6 @@ from force_bdss.io.workflow_reader import WorkflowReader
 from force_bdss.mco.parameters.base_mco_parameter import BaseMCOParameter
 from force_bdss.mco.parameters.base_mco_parameter_factory import \
     BaseMCOParameterFactory
-from force_bdss.mco.parameters.mco_parameter_factory_registry import \
-    MCOParameterFactoryRegistry
 
 try:
     import mock
@@ -37,9 +35,6 @@ class TestWorkflowWriter(unittest.TestCase):
         self.mock_registry.mco_bundle_by_id = mock.Mock(
             return_value=mock_mco_bundle)
 
-        self.mock_mco_parameter_registry = mock.Mock(
-            spec=MCOParameterFactoryRegistry)
-
     def test_write(self):
         wfwriter = WorkflowWriter()
         fp = StringIO()
@@ -58,8 +53,7 @@ class TestWorkflowWriter(unittest.TestCase):
         wf = self._create_mock_workflow()
         wfwriter.write(wf, fp)
         fp.seek(0)
-        wfreader = WorkflowReader(self.mock_registry,
-                                  self.mock_mco_parameter_registry)
+        wfreader = WorkflowReader(self.mock_registry)
         wf_result = wfreader.read(fp)
         self.assertEqual(wf_result.mco.bundle.id,
                          wf.mco.bundle.id)
@@ -74,7 +68,7 @@ class TestWorkflowWriter(unittest.TestCase):
             BaseMCOParameter(
                 factory=mock.Mock(
                     spec=BaseMCOParameterFactory,
-                    id=mco_parameter_id("enthought", "mock")
+                    id=mco_parameter_id("enthought", "mock", "mock")
                 )
             )
         ]

force_bdss/io/workflow_reader.py

@@ -3,8 +3,6 @@ import logging
 
 from traits.api import HasStrictTraits, Instance
 
-from ..mco.parameters.mco_parameter_factory_registry import (
-    MCOParameterFactoryRegistry)
 from ..bundle_registry_plugin import BundleRegistryPlugin
 from ..workspecs.workflow import Workflow
 
@@ -28,13 +26,8 @@ class WorkflowReader(HasStrictTraits):
     #: bundle-specific model objects.
     bundle_registry = Instance(BundleRegistryPlugin)
 
-    #: The registry for the MCO parameters. At the moment this
-    #: is not extensible via plugins as the one above.
-    mco_parameter_registry = Instance(MCOParameterFactoryRegistry)
-
     def __init__(self,
                  bundle_registry,
-                 mco_parameter_registry,
                  *args,
                  **kwargs):
         """Initializes the reader.
@@ -46,7 +39,6 @@ class WorkflowReader(HasStrictTraits):
             for a bundle identified by a given id.
         """
         self.bundle_registry = bundle_registry
-        self.mco_parameter_registry = mco_parameter_registry
 
         super(WorkflowReader, self).__init__(*args, **kwargs)
 
@@ -128,6 +120,7 @@ class WorkflowReader(HasStrictTraits):
         mco_bundle = registry.mco_bundle_by_id(mco_id)
         model_data = wf_data["mco"]["model_data"]
         model_data["parameters"] = self._extract_mco_parameters(
+            mco_id,
             model_data["parameters"])
         model = mco_bundle.create_model(
             wf_data["mco"]["model_data"])
@@ -182,7 +175,7 @@ class WorkflowReader(HasStrictTraits):
 
         return kpi_calculators
 
-    def _extract_mco_parameters(self, parameters_data):
+    def _extract_mco_parameters(self, mco_id, parameters_data):
         """Extracts the MCO parameters from the data as dictionary.
 
         Parameters
@@ -194,13 +187,13 @@ class WorkflowReader(HasStrictTraits):
         -------
         List of instances of a subclass of BaseMCOParameter
         """
-        registry = self.mco_parameter_registry
+        registry = self.bundle_registry
 
         parameters = []
 
         for p in parameters_data:
             id = p["id"]
-            factory = registry.get_factory_by_id(id)
+            factory = registry.mco_parameter_factory_by_id(mco_id, id)
             model = factory.create_model(p["model_data"])
             parameters.append(model)
 

force_bdss/mco/base_mco_bundle.py

@@ -68,3 +68,12 @@ class BaseMCOBundle(ABCHasStrictTraits):
         BaseMCOCommunicator
             An instance of the communicator
         """
+
+    @abc.abstractmethod
+    def parameter_factories(self):
+        """Returns the parameter factories supported by this MCO
+
+        Returns
+        -------
+        List of BaseMCOParameterFactory
+        """

force_bdss/mco/parameters/base_mco_parameter_factory.py

-from traits.has_traits import HasStrictTraits
-from traits.trait_types import String, Type
+from traits.api import HasStrictTraits, String, Type, Instance
+
+from ..base_mco_bundle import BaseMCOBundle
 
 
 class BaseMCOParameterFactory(HasStrictTraits):
     """Factory that produces the model instance of a given BASEMCOParameter
     instance.
 
-    Must be reimplemented for the specific parameter."""
+    Must be reimplemented for the specific parameter. The generic create_model
+    is generally enough, and the only entity to define is model_class with
+    the appropriate class of the parameter.
+    """
+
+    #: A reference to the bundle this parameter factory lives in.
+    bundle = Instance(BaseMCOBundle)
 
     #: A unique string identifying the parameter
     id = String()
@@ -20,8 +27,24 @@ class BaseMCOParameterFactory(HasStrictTraits):
     # The model class to instantiate when create_model is called.
     model_class = Type('BaseMCOParameter')
 
+    def __init__(self, bundle):
+        self.bundle = bundle
+        super(BaseMCOParameterFactory, self).__init__()
+
     def create_model(self, data_values=None):
         """Creates the instance of the model class and returns it.
+        You should not reimplement this, as the default is generally ok.
+        Instead, just define model_class with the appropriate Parameter class.
+
+        Parameters
+        ----------
+        data_values: dict or None
+            The dictionary of values for this parameter. If None, a default
+            object will be returned.
+
+        Returns
+        -------
+        instance of model_class.
         """
         if data_values is None:
             data_values = {}

force_bdss/mco/parameters/mco_parameter_factory_registry.py

-from traits.api import HasStrictTraits, Dict, String
-
-from force_bdss.mco.parameters.base_mco_parameter_factory import \
-    BaseMCOParameterFactory
-
-
-class MCOParameterFactoryRegistry(HasStrictTraits):
-    """Registry to keep the parameter factories and lookup them.
-    """
-    # Temp: this will become an extension point.
-    factories = Dict(String, BaseMCOParameterFactory)
-
-    def get_factory_by_id(self, id):
-        """Finds the factory by its id, so that we can obtain it as from
-        the id in the model file.
-        """
-        return self.factories[id]
-
-    def register(self, factory):
-        """Registers a new factory"""
-        self.factories[factory.id] = factory

force_bdss/mco/parameters/tests/test_base_mco_parameter_factory.py

 import unittest
+
+from force_bdss.mco.base_mco_bundle import BaseMCOBundle
+
+try:
+    import mock
+except ImportError:
+    from unittest import mock
 from traits.api import Int
 
 
@@ -20,7 +27,11 @@ class DummyMCOParameterFactory(BaseMCOParameterFactory):
 
 class TestBaseMCOParameterFactory(unittest.TestCase):
     def test_initialization(self):
-        factory = DummyMCOParameterFactory()
+        factory = DummyMCOParameterFactory(mock.Mock(spec=BaseMCOBundle))
         model = factory.create_model({"x": 42})
         self.assertIsInstance(model, DummyMCOParameter)
         self.assertEqual(model.x, 42)
+
+        model = factory.create_model()
+        self.assertIsInstance(model, DummyMCOParameter)
+        self.assertEqual(model.x, 0)

force_bdss/mco/parameters/tests/test_core_mco_parameters.py

-import unittest
-
-from force_bdss.mco.parameters import core_mco_parameters
-from force_bdss.mco.parameters.base_mco_parameter_factory import \
-    BaseMCOParameterFactory
-
-
-class TestCoreMCOParameters(unittest.TestCase):
-    def test_all_factories(self):
-        factories = core_mco_parameters.all_core_factories()
-        self.assertEqual(len(factories), 1)
-
-        for f in factories:
-            self.assertIsInstance(f, BaseMCOParameterFactory)

force_bdss/mco/parameters/tests/test_parameter_factory_registry.py

-import unittest
-
-from force_bdss.mco.parameters.mco_parameter_factory_registry import \
-    MCOParameterFactoryRegistry
-
-
-class TestParameterFactoryRegistry(unittest.TestCase):
-    def test_registry_init(self):
-        reg = MCOParameterFactoryRegistry()
-        self.assertEqual(reg.factories, {})