From 85641a7372050c4b7c7c6359d3d58a2bf646640f Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Thu, 24 Sep 2026 16:58:00 +0100 Subject: [PATCH 1/9] Make OptionsManager use petsctools.Options This means that we get the right option types when we read from petsctools.Options later on. --- petsctools/options.py | 26 ++++++++------------------ tests/test_options.py | 43 ++++++++++++++++++++++++++----------------- 2 files changed, 34 insertions(+), 35 deletions(-) diff --git a/petsctools/options.py b/petsctools/options.py index fe61931..1813e61 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -465,17 +465,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 +595,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 +605,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 = {} @@ -1136,6 +1122,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 @@ -1205,6 +1196,5 @@ def get( def getAll(self) -> dict[str, Any]: """Return all entries.""" return { - key: _global_appctx_data.get(value, value) - for key, value in super().getAll().items() + key: self[key] for key in super().getAll().keys() } diff --git a/tests/test_options.py b/tests/test_options.py index 4f4f1be..0d0a7e3 100644 --- a/tests/test_options.py +++ b/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) From dc0bd77b763acb1c92490eaa5c7565d30d8aad1a Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Thu, 24 Sep 2026 17:01:33 +0100 Subject: [PATCH 2/9] Remove AppContextManager --- petsctools/__init__.py | 1 - petsctools/options.py | 68 ++---------------------------------------- 2 files changed, 2 insertions(+), 67 deletions(-) diff --git a/petsctools/__init__.py b/petsctools/__init__.py index ace43f8..33fe705 100644 --- a/petsctools/__init__.py +++ b/petsctools/__init__.py @@ -55,7 +55,6 @@ def __getattr__(name): "get_commandline_options", "Options", "OptionsManager", - "AppContextManager", "petscobj2str", "attach_options", "has_options", diff --git a/petsctools/options.py b/petsctools/options.py index 1813e61..9fe519c 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -450,7 +450,6 @@ class OptionsManager: is_set_from_options inserted_options DefaultOptionSet - AppContextManager """ count = 0 @@ -632,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() @@ -646,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. @@ -899,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: @@ -1033,7 +970,6 @@ class Options(PETSc.Options): See Also -------- .OptionsManager - .AppContextManager petsc4py.PETSc.Options """ From 3d167499422da888f4b12c850a149881d1386dea Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Thu, 24 Sep 2026 17:12:01 +0100 Subject: [PATCH 3/9] fixup --- petsctools/options.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/petsctools/options.py b/petsctools/options.py index 9fe519c..e080f00 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -1131,6 +1131,11 @@ def get( def getAll(self) -> dict[str, Any]: """Return all entries.""" + # Extremely bizarrely we cannot do + # + # {key: self[key] for key, value in super().getAll()} + # + # because this will raise KeyErrors for some command line arguments. return { - key: self[key] for key in super().getAll().keys() + key: self.get(key, value) for key, value in super().getAll().items() } From 677338de76ffa76f7fd9f502c426a0339572f9fa Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Thu, 24 Sep 2026 17:28:43 +0100 Subject: [PATCH 4/9] linting --- petsctools/options.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/petsctools/options.py b/petsctools/options.py index e080f00..5876b4d 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -1137,5 +1137,6 @@ def getAll(self) -> dict[str, Any]: # # because this will raise KeyErrors for some command line arguments. return { - key: self.get(key, value) for key, value in super().getAll().items() + key: self.get(key, value) + for key, value in super().getAll().items() } From 64834726167fb8a5cd5d11d00cdb6441d28a0570 Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Fri, 9 Oct 2026 12:38:23 +0100 Subject: [PATCH 5/9] Remove unnecessary (?) hack --- petsctools/options.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/petsctools/options.py b/petsctools/options.py index 5876b4d..ce4edda 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -1131,12 +1131,7 @@ def get( def getAll(self) -> dict[str, Any]: """Return all entries.""" - # Extremely bizarrely we cannot do - # - # {key: self[key] for key, value in super().getAll()} - # - # because this will raise KeyErrors for some command line arguments. return { - key: self.get(key, value) + key: self[key] for key, value in super().getAll().items() } From 9264e845f6d26d01ebefc84e0f1a357f302c0f56 Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Fri, 9 Oct 2026 15:14:41 +0100 Subject: [PATCH 6/9] Apply batched suggestions from code review Co-authored-by: Josh Hope-Collins --- petsctools/options.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/petsctools/options.py b/petsctools/options.py index ce4edda..4e0a65d 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -1023,7 +1023,7 @@ def __getitem__(self, option: str | AppContextKey, /) -> Any: # A native type, try to perform a cast try: - opt_type = _option_types[f"{self.prefix or ''}{option}"] + opt_type = _option_types[f"{self.prefix}{option}"] except KeyError: # Option was not inserted using petsctools.Options, can't # do anything more @@ -1071,7 +1071,7 @@ def __setitem__(self, option: str | AppContextKey, value: Any, /) -> None: value = value_id # Save the type of value so we can cast to it in __getitem__ - _option_types[f"{self.prefix or ''}{option}"] = type(value) + _option_types[f"{self.prefix}{option}"] = type(value) super().__setitem__(option, value) From 71cd3446fff09ec0587bd5adafdacf7427cad813 Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Fri, 9 Oct 2026 15:15:30 +0100 Subject: [PATCH 7/9] Review feedback: fix+tests --- petsctools/options.py | 5 ++++- tests/test_options.py | 26 ++++++++++++++++++++++---- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/petsctools/options.py b/petsctools/options.py index 4e0a65d..7462a67 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -1097,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}{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}{option}"] super().__delitem__(option) def get( diff --git a/tests/test_options.py b/tests/test_options.py index 0d0a7e3..ca1d7c2 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -438,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() @@ -456,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"] From f7ed74f3e9b1bca6a83b531fd85e1d6b9044a7aa Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Fri, 9 Oct 2026 15:47:06 +0100 Subject: [PATCH 8/9] self.prefix can be None --- petsctools/options.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/petsctools/options.py b/petsctools/options.py index 7462a67..61a4600 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -990,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: """ @@ -1023,7 +1023,7 @@ def __getitem__(self, option: str | AppContextKey, /) -> Any: # A native type, try to perform a cast try: - opt_type = _option_types[f"{self.prefix}{option}"] + opt_type = _option_types[f"{self.prefix or ""}{option}"] except KeyError: # Option was not inserted using petsctools.Options, can't # do anything more @@ -1071,7 +1071,7 @@ def __setitem__(self, option: str | AppContextKey, value: Any, /) -> None: value = value_id # Save the type of value so we can cast to it in __getitem__ - _option_types[f"{self.prefix}{option}"] = type(value) + _option_types[f"{self.prefix or ""}{option}"] = type(value) super().__setitem__(option, value) @@ -1097,10 +1097,10 @@ def __delitem__(self, option: str | AppContextKey, /) -> None: value_id = super().__getitem__(option) del _global_appctx_data[value_id] - if f"{self.prefix}{option}" in _option_types: + 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}{option}"] + del _option_types[f"{self.prefix or ""}{option}"] super().__delitem__(option) def get( From 284d279ed920761dd0e2c4ff6850d7ca98aca8b9 Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Fri, 9 Oct 2026 15:47:43 +0100 Subject: [PATCH 9/9] fixup --- petsctools/options.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/petsctools/options.py b/petsctools/options.py index 61a4600..9bd8673 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -990,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 or "") + option)) + return AppContextKey(self.getString((self.prefix or '') + option)) def __getitem__(self, option: str | AppContextKey, /) -> Any: """ @@ -1023,7 +1023,7 @@ def __getitem__(self, option: str | AppContextKey, /) -> Any: # A native type, try to perform a cast try: - opt_type = _option_types[f"{self.prefix or ""}{option}"] + opt_type = _option_types[f"{self.prefix or ''}{option}"] except KeyError: # Option was not inserted using petsctools.Options, can't # do anything more @@ -1071,7 +1071,7 @@ def __setitem__(self, option: str | AppContextKey, value: Any, /) -> None: value = value_id # Save the type of value so we can cast to it in __getitem__ - _option_types[f"{self.prefix or ""}{option}"] = type(value) + _option_types[f"{self.prefix or ''}{option}"] = type(value) super().__setitem__(option, value) @@ -1097,10 +1097,10 @@ def __delitem__(self, option: str | AppContextKey, /) -> None: value_id = super().__getitem__(option) del _global_appctx_data[value_id] - if f"{self.prefix or ""}{option}" in _option_types: + 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}"] + del _option_types[f"{self.prefix or ''}{option}"] super().__delitem__(option) def get(