From b0cebcffe1e8975b5a4c3028c8720743325f30bd Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Fri, 16 Oct 2026 22:52:16 +0100 Subject: [PATCH 1/3] Add hacky fix for buggy Options().getAll() --- petsctools/options.py | 7 ++++++- tests/test_options.py | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/petsctools/options.py b/petsctools/options.py index ecd715a..92a09e0 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -1126,7 +1126,12 @@ def get( def getAll(self) -> dict[str, Any]: """Return all entries.""" + # Try to apply type casting to the objects returned by super().getAll() + # but use .get instead of __getitem__ because the latter does not work + # with command line arguments starting with '--'. For example, '--pdb' + # will be shown by getAll() as the key 'pdb', PETSc.Options()["pdb"] + # will raise a KeyError. return { - key: self[key] + key: self.get(key, value) for key, value in super().getAll().items() } diff --git a/tests/test_options.py b/tests/test_options.py index ca1d7c2..3298886 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -20,6 +20,8 @@ def temporarily_remove_options(): options = petsctools.Options() previous_options = dict(options.getAll()) + # Note that options.clear() does not actually remove all options from + # the database - definitely a bug. options.clear() previous_option_types = petsctools.options._option_types petsctools.options._option_types = {} @@ -28,6 +30,7 @@ def temporarily_remove_options(): yield + options.clear() for k, v in previous_options.items(): options[k] = v petsctools.options._option_types = previous_option_types @@ -481,3 +484,26 @@ def test_options_missing_types(setter, deleter): for opts in [petsc_opts, petsctools_opts]: with pytest.raises(KeyError): opts["my_option"] + + +@pytest.mark.skipnopetsc4py +@pytest.mark.parametrize("backend", ["petsc4py", "petsctools"]) +def test_double_dash_options_do_not_break_getAll(backend): + # PETSc.Options.getAll() does strange things to double-dashed options. Here + # we just check that the original behaviour is retained and that we do not + # raise KeyErrors. + from petsc4py import PETSc + + opts = petsctools.Options() if backend == "petsctools" else PETSc.Options() + opts["--value"] = "xyz" + + # Note that opts.getAll() contains more than one entry because + # PETSc.Options().clear() is bugged so we get previous entries too. + assert ("value", "xyz") in opts.getAll().items() + + # Check that PETSc have the odd behaviour that we expect - if this fails + # then we can undo things! + with pytest.raises(KeyError): + opts["value"] + with pytest.raises(KeyError): + opts.getString("value") From 0105a3ed2e598fed7ba21ea93b418976cc91c00d Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Mon, 19 Oct 2026 14:07:03 +0100 Subject: [PATCH 2/3] Update petsctools/options.py Co-authored-by: Josh Hope-Collins --- petsctools/options.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/petsctools/options.py b/petsctools/options.py index 92a09e0..554d9e2 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -1126,11 +1126,12 @@ def get( def getAll(self) -> dict[str, Any]: """Return all entries.""" - # Try to apply type casting to the objects returned by super().getAll() - # but use .get instead of __getitem__ because the latter does not work - # with command line arguments starting with '--'. For example, '--pdb' - # will be shown by getAll() as the key 'pdb', PETSc.Options()["pdb"] - # will raise a KeyError. + # Try to apply type casting to the objects returned by super().getAll(). + # Command line arguments with double dashes e.g. "--opt val" will + # appear as {"opt": "val"} in PETSc.Options().getAll(), but PETSc.Options()["opt"] + # or PETSc.Options().get("opt") will raise KeyErrors. + # Passing self.get a default value from PETSc.Options().getAll() works + # around this error by returning the default when the KeyError is raised. return { key: self.get(key, value) for key, value in super().getAll().items() From 49233450e7cc68592bffadc772fb9f460ed89d7f Mon Sep 17 00:00:00 2001 From: Connor Ward Date: Mon, 19 Oct 2026 14:10:30 +0100 Subject: [PATCH 3/3] Improve comments and add extra getAll test --- petsctools/options.py | 13 +++++++------ tests/test_options.py | 32 +++++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/petsctools/options.py b/petsctools/options.py index 554d9e2..0bbe4da 100644 --- a/petsctools/options.py +++ b/petsctools/options.py @@ -1126,12 +1126,13 @@ def get( def getAll(self) -> dict[str, Any]: """Return all entries.""" - # Try to apply type casting to the objects returned by super().getAll(). - # Command line arguments with double dashes e.g. "--opt val" will - # appear as {"opt": "val"} in PETSc.Options().getAll(), but PETSc.Options()["opt"] - # or PETSc.Options().get("opt") will raise KeyErrors. - # Passing self.get a default value from PETSc.Options().getAll() works - # around this error by returning the default when the KeyError is raised. + # Try to apply type casting to the objects returned by + # super().getAll(). Command line arguments with double dashes e.g. + # "--opt val" will appear as {"opt": "val"} in + # PETSc.Options().getAll(), but PETSc.Options()["opt"] or + # PETSc.Options().get("opt") will raise KeyErrors. Passing self.get() a + # default value from PETSc.Options().getAll() works around this error + # by returning the default when the KeyError is raised. return { key: self.get(key, value) for key, value in super().getAll().items() diff --git a/tests/test_options.py b/tests/test_options.py index 3298886..1d9fcac 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -486,19 +486,41 @@ def test_options_missing_types(setter, deleter): opts["my_option"] +@pytest.mark.skipnopetsc4py +def test_getAll_does_type_casting(): + opts = petsctools.Options() + params = { + "integer": 666, + "float": 12.3, + "string": "abc", + "bool": False, + } + for name, value in params.items(): + opts[name] = value + + # Note that opts.getAll() can contain more than one entry because + # PETSc.Options().clear() is bugged, so we get contamination from previous + # tests. + for name, value in params.items(): + assert (name, value) in opts.getAll().items() + + @pytest.mark.skipnopetsc4py @pytest.mark.parametrize("backend", ["petsc4py", "petsctools"]) def test_double_dash_options_do_not_break_getAll(backend): - # PETSc.Options.getAll() does strange things to double-dashed options. Here - # we just check that the original behaviour is retained and that we do not - # raise KeyErrors. + # PETSc.Options.getAll() will include options passed with double-dashes + # (e.g. --pdb as 'pdb') but any attempt to access the option with + # .get(), __getitem__ etc will raise a KeyError. This test is here to + # check that petsctools.Options.getAll() will run without error when '--' + # options are passed (even though the result itself is bugged). from petsc4py import PETSc opts = petsctools.Options() if backend == "petsctools" else PETSc.Options() opts["--value"] = "xyz" - # Note that opts.getAll() contains more than one entry because - # PETSc.Options().clear() is bugged so we get previous entries too. + # Note that opts.getAll() can contain more than one entry because + # PETSc.Options().clear() is bugged, so we get contamination from previous + # tests. assert ("value", "xyz") in opts.getAll().items() # Check that PETSc have the odd behaviour that we expect - if this fails