From 3c3ac9a12a190b5083f16dd9b15106cbb862bd65 Mon Sep 17 00:00:00 2001
From: Pascal Bourgault <bourgault.pascal@ouranos.ca>
Date: Mon, 16 Oct 2023 13:27:44 -0400
Subject: [PATCH] Fix AttrFormatter  - rewrite with clearer code

---
 tests/test_indicators.py |  2 +-
 xclim/core/formatting.py | 34 ++++++++++++++++++++++++----------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/tests/test_indicators.py b/tests/test_indicators.py
index 6ce3dffc9..14ee9b5a2 100644
--- a/tests/test_indicators.py
+++ b/tests/test_indicators.py
@@ -592,7 +592,7 @@ def test_AttrFormatter():
     # Missing mod:
     assert fmt.format("{adj}", adj="evil") == "méchant"
     # Mod with unknown value
-    with pytest.raises(ValueError):
+    with pytest.warns(match="Requested formatting `m` for unknown string `funny`."):
         fmt.format("{adj:m}", adj="funny")
 
 
diff --git a/xclim/core/formatting.py b/xclim/core/formatting.py
index 8d9c22958..c11414661 100644
--- a/xclim/core/formatting.py
+++ b/xclim/core/formatting.py
@@ -6,9 +6,9 @@
 
 import datetime as dt
 import itertools
-import logging
 import re
 import string
+import warnings
 from ast import literal_eval
 from fnmatch import fnmatch
 from inspect import _empty, signature  # noqa
@@ -124,18 +124,30 @@ def format_field(self, value, format_spec):
         'La moyenne annuelle est faite sur un échantillon mensuel'
         """
         baseval = self._match_value(value)
-        if baseval is not None and not format_spec:
+        if baseval is None:  # Not something we know how to translate
+            if format_spec in self.modifiers + [
+                "r"
+            ]:  # Woops, however a known format spec was asked
+                warnings.warn(
+                    f"Requested formatting `{format_spec}` for unknown string `{value}`."
+                )
+                format_spec = ""
+            return super().format_field(value, format_spec)
+        # Thus, known value
+
+        if not format_spec:  # (None or '') No modifiers, return first
             return self.mapping[baseval][0]
 
-        if format_spec in self.modifiers:
-            if baseval is not None:
-                return self.mapping[baseval][self.modifiers.index(format_spec)]
-            raise ValueError(
-                f"No known mapping for string '{value}' with modifier '{format_spec}'"
-            )
-        if format_spec == "r":
+        if format_spec == "r":  # Raw modifier
             return super().format_field(value, "")
-        return super().format_field(value, format_spec)
+
+        if format_spec in self.modifiers:  # Known modifier
+            if len(self.mapping[baseval]) == 1:  # But unmodifiable entry
+                return self.mapping[baseval][0]
+            # Known modifier, modifiable entry
+            return self.mapping[baseval][self.modifiers.index(format_spec)]
+        # Known value but unknown modifier, must be a built-in one, only works for the default val...
+        return super().format_field(self.mapping[baseval][0], format_spec)
 
     def _match_value(self, value):
         if isinstance(value, str):
@@ -181,6 +193,8 @@ def _match_value(self, value):
         "var": ["variance"],
         "absamp": ["absolute amplitude"],
         "relamp": ["relative amplitude"],
+        # For when we are formatting indicator classes with empty options
+        "<class 'inspect._empty'>": ["<empty>"],
     },
     ["adj", "noun"],
 )