Post petsctools.Options improvements to OptionsManager

petsctools/__init__.py

@@ -55,7 +55,6 @@ def __getattr__(name):
             "get_commandline_options",
             "Options",
             "OptionsManager",
-            "AppContextManager",
             "petscobj2str",
             "attach_options",
             "has_options",

petsctools/options.py

@@ -450,7 +450,6 @@ class OptionsManager:
     is_set_from_options
     inserted_options
     DefaultOptionSet
-    AppContextManager
     """
 
     count = 0
@@ -465,17 +464,6 @@ def __init__(self, parameters: dict[str, Any],
             # Convert nested dicts
             parameters = flatten_parameters(parameters)
 
-        # Replace any Python objects in the parameters dict with appctx entries
-        appmngr = AppContextManager()
-        for key, value in parameters.items():
-            # Convert Python objects into their string representation for
-            # things like 'pc_python_type' and 'snes_python_type'
-            if key.endswith("python_type") and isinstance(value, type):
-                parameters[key] = f"{value.__module__}.{value.__name__}"
-            elif not isinstance(value, _native_petsc_option_types):
-                parameters[key] = appmngr.add(value)
-        self.appmngr = appmngr
-
         # If no prefix is provided generate a default prefix
         if options_prefix is None:
             default_prefix = default_prefix or "petsctools_"
@@ -606,8 +594,7 @@ def inserted_options(self):
         try:
             for k, v in self.parameters.items():
                 self.options_object[self.options_prefix + k] = v
-            with self.appmngr.inserted_appctx():
-                yield
+            yield
         finally:
             for k in self.parameters:
                 if self.options_object.used(self.options_prefix + k):
@@ -617,11 +604,9 @@ def inserted_options(self):
 
     @functools.cached_property
     def options_object(self):
-        from petsc4py import PETSc
-
         # We can't pass the prefix here because that doesn't DTRT
         # for flag options
-        return PETSc.Options()
+        return Options()
 
 
 _option_types = {}
@@ -646,11 +631,11 @@ class AppContextKey(str):
     Warning
     -------
     This type should not be instantiated directly by the user, it
-    will be generated as needed by the :class:`.AppContextManager`.
+    will be generated as needed by the :class:`.Options`.
 
     See Also
     --------
-    .AppContextManager
+    .Options
     """
 
     _count = itertools.count()
@@ -660,67 +645,6 @@ def _generate_key(cls):
         return f"{_APPCTX_KEY_PREFIX}{next(cls._count)}"
 
 
-class AppContextManager:
-    """Class for storing Python data associated with a particular PETSc object.
-
-    See Also
-    --------
-    .OptionsManager
-    .Options
-    petsc4py.PETSc.Options
-
-    """
-
-    def __init__(self):
-        self._data = {}
-
-    def add(self, val: Any) -> AppContextKey:
-        """
-        Add a value to be inserted into the global app context database
-        by the :func:`.inserted_options` context manager, or
-        by the ``AppContextManager`` directly with
-        :meth:`.AppContextManager.inserted_appctx`.
-
-        The autogenerated key returned by this method must only be used
-        as the value for the corresponding entry in the parameters dictionary
-        passed to :func:`.set_from_options`.
-
-        Parameters
-        ----------
-        val
-            The value to be inserted into the ``AppContext``.
-
-        Returns
-        -------
-        AppContextKey
-            The key to put into the ``PETSc.Options`` dictionary.
-        """
-        key = AppContextKey._generate_key()
-        self._data[key] = val
-        return key
-
-    @contextlib.contextmanager
-    def inserted_appctx(self):
-        """Context manager inside which the global options
-        database contains the entries added to this ``AppContextManager``.
-        """
-        # We don't overwrite existing entries in the global data,
-        # so we need to keep track of what we do actually put in
-        # so we don't accidentally remove something we shouldn't.
-        to_delete = set()
-        try:
-            for k, v in self._data.items():
-                if k not in _global_appctx_data:
-                    _global_appctx_data[k] = v
-                    to_delete.add(k)
-            yield
-        finally:
-            for k in self._data:
-                if k in to_delete:
-                    del _global_appctx_data[k]
-                    to_delete.remove(k)
-            assert len(to_delete) == 0
-
 
 def petscobj2str(obj: petsc4py.PETSc.Object) -> str:
     """Return a string with a PETSc object type and prefix.
@@ -913,7 +837,6 @@ def set_from_options(
     OptionsManager.set_from_options
     attach_options
     DefaultOptionSet
-    AppContextManager
     """
     if has_options(obj):
         if parameters is not None or options_prefix is not None:
@@ -1047,7 +970,6 @@ class Options(PETSc.Options):
     See Also
     --------
     .OptionsManager
-    .AppContextManager
     petsc4py.PETSc.Options
 
     """
@@ -1068,7 +990,7 @@ def _appctx_key_from_option(self, option: str) -> AppContextKey:
         key
             An internal key corresponding to ``option``.
         """
-        return AppContextKey(self.getString(self.prefix + option))
+        return AppContextKey(self.getString((self.prefix or '') + option))
 
     def __getitem__(self, option: str | AppContextKey, /) -> Any:
         """
@@ -1136,6 +1058,11 @@ def __setitem__(self, option: str | AppContextKey, value: Any, /) -> None:
             The value to insert.
 
         """
+        # Convert Python objects into their string representation for
+        # things like 'pc_python_type' and 'snes_python_type'
+        if option.endswith("python_type") and isinstance(value, type):
+            value = f"{value.__module__}.{value.__name__}"
+
         if not isinstance(value, _native_petsc_option_types):
             # value cannot be natively stored, put in the global appctx data
             # and pass an ID to the options database
@@ -1170,7 +1097,10 @@ def __delitem__(self, option: str | AppContextKey, /) -> None:
             value_id = super().__getitem__(option)
             del _global_appctx_data[value_id]
 
-        del _option_types[f"{self.prefix or ''}{option}"]
+        if f"{self.prefix or ''}{option}" in _option_types:
+            # If the option was set using PETSc.Options then there won't be a
+            # corresponding _option_types entry
+            del _option_types[f"{self.prefix or ''}{option}"]
         super().__delitem__(option)
 
     def get(
@@ -1205,6 +1135,6 @@ def get(
     def getAll(self) -> dict[str, Any]:
         """Return all entries."""
         return {
-            key: _global_appctx_data.get(value, value)
+            key: self[key]
             for key, value in super().getAll().items()
         }

tests/test_options.py

@@ -7,22 +7,31 @@
 import petsctools
 
 
-@pytest.fixture(autouse=True, scope="module")
+@pytest.fixture(autouse=True)
 def temporarily_remove_options():
     """Remove all options when the module is entered and reinsert them at exit.
     This ensures that options in e.g. petscrc files will not pollute the tests.
     """
-    if petsctools.PETSC4PY_INSTALLED:
-        PETSc = petsctools.init()
-        options = PETSc.Options()
-        previous_options = {
-            k: v for k, v in options.getAll().items()
-        }
-        options.clear()
+    if not petsctools.PETSC4PY_INSTALLED:
+        yield
+        return
+
+    petsctools.init([])
+    options = petsctools.Options()
+
+    previous_options = dict(options.getAll())
+    options.clear()
+    previous_option_types = petsctools.options._option_types
+    petsctools.options._option_types = {}
+    previous_global_appctx_data = petsctools.options._global_appctx_data
+    petsctools.options._global_appctx_data = {}
+
     yield
-    if petsctools.PETSC4PY_INSTALLED:
-        for k, v in previous_options.items():
-            options[k] = v
+
+    for k, v in previous_options.items():
+        options[k] = v
+    petsctools.options._option_types = previous_option_types 
+    petsctools.options._global_appctx_data = previous_global_appctx_data 
 
 
 @pytest.fixture(autouse=True)
@@ -181,8 +190,6 @@ def test_default_options():
 
 @pytest.mark.skipnopetsc4py
 def test_python_options_with_manager():
-    petsctools.init()
-
     prefix0_param = object()
     prefix1_param = object()
     opts_manager = petsctools.OptionsManager(
@@ -201,19 +208,20 @@ def test_python_options_with_manager():
     with opts_manager.inserted_options():
         assert opts0.get("param1") is prefix0_param
         assert opts0["param1"] is prefix0_param
+        assert opts0["param2"] == "some_value"
         assert opts0.getAll() \
             == {"param1": prefix0_param, "param2": "some_value"}
 
         assert opts1.get("param1") is prefix1_param
         assert opts1["param1"] is prefix1_param
-        # NOTE: ideally we would get the integer back here
+        assert opts1["param2"] == 666
         assert opts1.getAll() \
-            == {"param1": prefix1_param, "param2": "666"}
+            == {"param1": prefix1_param, "param2": 666}
 
 
 @pytest.mark.skipnopetsc4py
 def test_python_options_without_manager():
-    PETSc = petsctools.init()
+    from petsc4py import PETSc
 
     petsc_opts = PETSc.Options()
     petsctools_opts = petsctools.Options()
@@ -267,7 +275,8 @@ def apply(self, pc, x, y):
 @pytest.mark.parametrize("use_prefix", ["with_prefix", "without_prefix"])
 @pytest.mark.parametrize("use_pc_class", [False, True])
 def test_python_options_ksp(use_prefix, use_pc_class):
-    PETSc = petsctools.init()
+    from petsc4py import PETSc
+
     n = 4
     sizes = (n, n)
 
@@ -429,9 +438,12 @@ def test_options_preserve_types():
 
 
 @pytest.mark.skipnopetsc4py
-def test_options_missing_types():
+@pytest.mark.parametrize("deleter", ["petsc4py", "petsctools"])
+@pytest.mark.parametrize("setter", ["petsc4py", "petsctools"])
+def test_options_missing_types(setter, deleter):
     # Test that options inserted using PETSc.Options instead of
-    # petsctools.Options still work even though we don't know the type
+    # petsctools.Options still work even though we don't know the type.
+    # Also test the inverse.
     from petsc4py import PETSc
 
     petsc_opts = PETSc.Options()
@@ -447,10 +459,25 @@ def test_options_missing_types():
         (None, None),  # no suitable default getter for None
     ]
     for item, getter in items:
-        petsc_opts["my_option"] = item
+        if setter == "petsc4py":
+            petsc_opts["my_option"] = item
+        else:
+            petsctools_opts["my_option"] = item
 
         assert isinstance(petsc_opts["my_option"], str)
-        assert isinstance(petsctools_opts["my_option"], str)
+        if setter == "petsc4py":
+            assert isinstance(petsctools_opts["my_option"], str)
+        else:
+            assert petsctools_opts["my_option"] == item
 
         if getter is not None:
             assert getter("my_option") == item
+
+        # also test removal
+        if deleter == "petsc4py":
+            del petsc_opts["my_option"]
+        else:
+            del petsctools_opts["my_option"]
+        for opts in [petsc_opts, petsctools_opts]:
+            with pytest.raises(KeyError):
+                opts["my_option"]