From e6d9fb1dee6ab80fd957613d66aaf4307d9dd744 Mon Sep 17 00:00:00 2001
From: David Fairbrother <DavidFair@users.noreply.github.com>
Date: Mon, 20 Mar 2017 16:39:10 +0000
Subject: [PATCH] Re #19156 Print allowed values on missing enum param

---
 .../routines/InstrumentSettings.py            | 51 ++++++++++++++-----
 .../isis_powder/routines/RunDetails.py        |  2 +-
 .../test/ISISPowderInstrumentSettingsTest.py  | 10 ++++
 3 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/scripts/Diffraction/isis_powder/routines/InstrumentSettings.py b/scripts/Diffraction/isis_powder/routines/InstrumentSettings.py
index 37bf30cc821..97d08f448f7 100644
--- a/scripts/Diffraction/isis_powder/routines/InstrumentSettings.py
+++ b/scripts/Diffraction/isis_powder/routines/InstrumentSettings.py
@@ -32,17 +32,16 @@ class InstrumentSettings(object):
     #  were going to throw at this point unless the attribute was optional.
     def __getattr__(self, item):
         # Check if it is in our parameter mapping
-        is_known_internally = next((param_entry for param_entry in self._param_map if item == param_entry.int_name), None)
+        known_param = next((param_entry for param_entry in self._param_map if item == param_entry.int_name), None)
 
-        if is_known_internally:
-            if is_known_internally.optional:
+        if known_param:
+            if known_param.optional:
                 # Optional param return none
                 return None
             else:
                 # User forgot to enter the param:
-                raise AttributeError(
-                    "The parameter with name: '" + str(is_known_internally.ext_name) + "' is required but "
-                    "was not set or passed.\nPlease set this configuration option and try again")
+                self._raise_user_param_missing_error(known_param)
+
         else:
             # If you have got here from a grep or something similar this error message means the line caller
             # has asked for a class attribute which does not exist. These attributes are set in a mapping file which
@@ -92,6 +91,19 @@ class InstrumentSettings(object):
                 _print_known_keys(self._param_map)
                 raise ValueError("Unknown configuration key: " + str(config_key))
 
+    @staticmethod
+    def _raise_user_param_missing_error(param_entry):
+        err_text = "The parameter with name: '" + str(param_entry.ext_name) + "' is required but "
+        err_text += "was not set or passed.\n"
+        # If this item is an enum print known values
+        if param_entry.enum_class:
+            known_vals = _get_enum_values(param_entry.enum_class)
+            err_text += "Acceptable values for this parameter are: " + str(known_vals[0])
+            for val in known_vals[1:]:
+                err_text += ", " + str(val)
+
+        raise AttributeError(err_text)
+
     def _update_attribute(self, param_map, param_val, suppress_warnings):
         attribute_name = param_map.int_name
 
@@ -126,18 +138,14 @@ def _check_value_is_in_enum(val, enum):
     enum_known_vals = []
     lower_string_val = str(val).lower()
 
-    for k, enum_val in iteritems(enum.__dict__):
-        # Get all class attribute and value pairs except enum_friendly_name
-        if k.startswith("__") or k.lower() == "enum_friendly_name":
-            continue
-
-        enum_known_vals.append(enum_val)
+    known_values = _get_enum_values(enum_cls=enum)
+    for enum_val in known_values:
 
         if lower_string_val == enum_val.lower():
             # Get the correctly capitalised value so we no longer have to call lower
             val = enum_val
             seen_val_in_enum = True
-            # Have to keep looping here in case we hit the err so all known keys are added
+            break
 
     # Check to see if the value was seen
     if seen_val_in_enum:
@@ -152,6 +160,23 @@ def _check_value_is_in_enum(val, enum):
         raise ValueError(e_msg)
 
 
+def _get_enum_values(enum_cls):
+    """
+    Gets all acceptable values for the specified enum class and returns them as a list
+    :param enum_cls: The enum to process
+    :return: List of accepted values for this enum
+    """
+    enum_known_vals = []
+
+    for k, enum_val in iteritems(enum_cls.__dict__):
+        # Get all class attribute and value pairs except enum_friendly_name
+        if k.startswith("__") or k.lower() == "enum_friendly_name":
+            continue
+        enum_known_vals.append(enum_val)
+
+    return enum_known_vals
+
+
 def _print_known_keys(master_mapping):
     print ("\nKnown keys are:")
     print("----------------------------------")
diff --git a/scripts/Diffraction/isis_powder/routines/RunDetails.py b/scripts/Diffraction/isis_powder/routines/RunDetails.py
index 698aefb6fe3..c1e07a8417a 100644
--- a/scripts/Diffraction/isis_powder/routines/RunDetails.py
+++ b/scripts/Diffraction/isis_powder/routines/RunDetails.py
@@ -15,7 +15,7 @@ def create_run_details_object(run_number_string, inst_settings, is_vanadium_run,
     calibration_dir = os.path.normpath(os.path.expanduser(inst_settings.calibration_dir))
     label = common.cal_map_dictionary_key_helper(dictionary=cal_map_dict, key="label")
     offset_file_name = common.cal_map_dictionary_key_helper(dictionary=cal_map_dict, key="offset_file_name")
-    
+
     # Always make sure the offset file name is included
     if splined_name_list:
         splined_name_list.append(offset_file_name)
diff --git a/scripts/test/ISISPowderInstrumentSettingsTest.py b/scripts/test/ISISPowderInstrumentSettingsTest.py
index a59eda9c996..0376e0e2f10 100644
--- a/scripts/test/ISISPowderInstrumentSettingsTest.py
+++ b/scripts/test/ISISPowderInstrumentSettingsTest.py
@@ -20,6 +20,16 @@ class ISISPowderInstrumentSettingsTest(unittest.TestCase):
             foo = inst_settings_obj.script_facing_name
             del foo
 
+    def test_user_missing_attribute_prints_enum_values(self):
+        param_entry = ParamMapEntry.ParamMapEntry(ext_name="user_facing_name", int_name="script_facing_name",
+                                                  enum_class=SampleEnum)
+        inst_settings_obj = InstrumentSettings.InstrumentSettings(param_map=[param_entry])
+
+        # Check it still prints the acceptable values when it fails
+        with assertRaisesRegex(self, AttributeError, "a foo, A BAR"):
+            foo = inst_settings_obj.script_facing_name
+            del foo
+
     def test_developer_missing_attribute_is_detected(self):
         param_entry = ParamMapEntry.ParamMapEntry(ext_name="user_facing_name", int_name="script_facing_name")
 
-- 
GitLab