From 83741724791aaca0578f9e1b3796985e2e4b2189 Mon Sep 17 00:00:00 2001
From: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>
Date: Fri, 13 Dec 2024 11:49:09 -0800
Subject: [PATCH] add warning for unsupported settings yaml parameters (#1051)

* adding warning and test

* be more precise about allowed fields handling

* using more explicit set notation

* add rever file
---
 news/settings_yaml_warning.rst                | 23 +++++++++++++++++++
 openfecli/parameters/plan_network_options.py  | 20 ++++++++--------
 .../parameters/test_plan_network_options.py   | 19 +++++++++++++++
 3 files changed, 53 insertions(+), 9 deletions(-)
 create mode 100644 news/settings_yaml_warning.rst

diff --git a/news/settings_yaml_warning.rst b/news/settings_yaml_warning.rst
new file mode 100644
index 000000000..d9e7f73c9
--- /dev/null
+++ b/news/settings_yaml_warning.rst
@@ -0,0 +1,23 @@
+**Added:**
+
+* CLI setup will raise warnings for unsupported top-level fields.
+
+**Changed:**
+
+* <news item>
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>
diff --git a/openfecli/parameters/plan_network_options.py b/openfecli/parameters/plan_network_options.py
index e486973f5..5ae0dc496 100644
--- a/openfecli/parameters/plan_network_options.py
+++ b/openfecli/parameters/plan_network_options.py
@@ -70,15 +70,17 @@ def parse_yaml_planner_options(contents: str) -> CliYaml:
     """
     raw = yaml.safe_load(contents)
 
-    if False:
-        # todo: warnings about extra fields we don't expect?
-        expected = {'mapper', 'network'}
-        for field in raw:
-            if field in expected:
-                continue
-            warnings.warn(f"Ignoring unexpected section: '{field}'")
+    expected_fields = {'mapper', 'network'}
+    present_fields = set(raw.keys())
+    usable_fields = present_fields.intersection(expected_fields)
+    ignored_fields = present_fields.difference(expected_fields)
 
-    return CliYaml(**raw)
+    for field in ignored_fields:
+        warnings.warn(f"Ignoring unexpected section: '{field}'")
+
+    filtered = {k:raw[k] for k in usable_fields}
+
+    return CliYaml(**filtered)
 
 
 def load_yaml_planner_options(path: Optional[str], context) -> PlanNetworkOptions:
@@ -185,7 +187,7 @@ def load_yaml_planner_options(path: Optional[str], context) -> PlanNetworkOption
 Currently it can contain sections for customising the
 atom mapper and network planning algorithm,
 these are addressed using a `mapper:` or `network:` key in the yaml file.
-The algorithm to be used for these sections is then specified by the `method:` key. 
+The algorithm to be used for these sections is then specified by the `method:` key.
 For choosing mappers, either the LomapAtomMapper or KartografAtomMapper are allowed choices,
 while for the network planning algorithm either the generate_minimal_spanning_tree or
 generate_minimal_redundant_network options are allowed.
diff --git a/openfecli/tests/parameters/test_plan_network_options.py b/openfecli/tests/parameters/test_plan_network_options.py
index ae69b605f..63cd68674 100644
--- a/openfecli/tests/parameters/test_plan_network_options.py
+++ b/openfecli/tests/parameters/test_plan_network_options.py
@@ -35,6 +35,15 @@ def partial_network_yaml():
     scorer: default_lomap_scorer
 """
 
+@pytest.fixture()
+def unsupported_field_yaml():
+    return """\
+protocol:
+  settings:
+    forcefield_settings:
+      small_molecule_forcefield: 'espaloma-0.2.2'
+    protocol_repeats: 2
+"""
 
 def test_loading_full_yaml(full_yaml):
     d = plan_network_options.parse_yaml_planner_options(full_yaml)
@@ -64,3 +73,13 @@ def test_loading_network_yaml(partial_network_yaml):
     assert d.network
     assert d.network.method == 'generate_radial_network'
     assert d.network.settings['scorer'] == 'default_lomap_scorer'
+
+def test_raise_unsupported_fields_warning(full_yaml, unsupported_field_yaml):
+    with pytest.warns(UserWarning, match='Ignoring unexpected section:'):
+      d = plan_network_options.parse_yaml_planner_options(full_yaml + unsupported_field_yaml)
+
+    assert d.mapper
+    assert d.mapper.method == 'LomapAtomMapper'.lower()
+    assert d.mapper.settings['timeout'] == 120
+    assert d.network
+    assert d.network.method == 'generate_radial_network'