Repository: 12rambau/sepal_ui

Issue:
refactor check_input to raise error instead of displaying a message?
https://github.com/12rambau/sepal_ui/blob/a9255e7c566aac31ee7f8303e74fb7e8a3d57e5f/sepal_ui/sepalwidgets/alert.py#L232

This was initially coded as a message to prevent the execution from stopping when a function was using undefined variables. Along the way we introduced `try  ... exept` statements in our functions and now main methods are mostly used with the `loading_button` decorator. 

I think we could refactor the `check_input` so that it directly raises an error instead writing down a message in the alert. Maybe even better, we deprecate this function and we move `check_input` to `utils` (with the error raising mechanism). 

Discussion:

Patch:
diff --git a/sepal_ui/message/en/utils.json b/sepal_ui/message/en/utils.json
new file mode 100644
index 00000000..0282d86c
--- /dev/null
+++ b/sepal_ui/message/en/utils.json
@@ -0,0 +1,7 @@
+{
+    "utils": {
+        "check_input": {
+            "error": "The value has not been initialized"
+        }
+    }
+}
\ No newline at end of file
diff --git a/sepal_ui/scripts/utils.py b/sepal_ui/scripts/utils.py
index b062906d..1f37abc2 100644
--- a/sepal_ui/scripts/utils.py
+++ b/sepal_ui/scripts/utils.py
@@ -17,6 +17,7 @@ from matplotlib import colors as c
 from deprecated.sphinx import versionadded, deprecated
 
 import sepal_ui
+from sepal_ui.message import ms
 from sepal_ui.conf import config_file, config
 from .warning import SepalWarning
 
@@ -631,3 +632,31 @@ def geojson_to_ee(geo_json, geodesic=False, encoding="utf-8"):
         raise ValueError("Could not convert the geojson to ee.Geometry()")
 
     return
+
+
+def check_input(input_, msg=ms.utils.check_input.error):
+    """
+    Check if the inpupt value is initialized.
+    If not raise an error, else return True
+
+    Args:
+        input\_ (any): the input to check
+        msg (str, optionnal): the message to display if the input is not set
+
+    Return:
+        (bool): check if the value is initialized
+    """
+
+    # by the default the variable is considered valid
+    init = True
+
+    # check the collection type that are the only one supporting the len method
+    try:
+        init = False if len(input_) == 0 else init
+    except Exception:
+        init = False if input_ is None else init
+
+    if init is False:
+        raise ValueError(msg)
+
+    return init
diff --git a/sepal_ui/sepalwidgets/alert.py b/sepal_ui/sepalwidgets/alert.py
index 4fd9c633..55ad3b65 100644
--- a/sepal_ui/sepalwidgets/alert.py
+++ b/sepal_ui/sepalwidgets/alert.py
@@ -3,11 +3,13 @@ from tqdm.notebook import tqdm
 from ipywidgets import jslink, Output
 import ipyvuetify as v
 from traitlets import Unicode, observe, directional_link, Bool
+from deprecated.sphinx import deprecated
 
 from sepal_ui.sepalwidgets.sepalwidget import SepalWidget
 from sepal_ui.scripts.utils import set_type
 from sepal_ui.frontend.styles import TYPES, color
 from sepal_ui.message import ms
+from sepal_ui.scripts import utils as su
 
 __all__ = ["Divider", "Alert", "StateBar", "Banner"]
 
@@ -229,6 +231,7 @@ class Alert(v.Alert, SepalWidget):
 
         return self
 
+    @deprecated(version="3.0", reason="This method is now part of the utils module")
     def check_input(self, input_, msg=None):
         """
         Check if the inpupt value is initialized.
@@ -241,20 +244,9 @@ class Alert(v.Alert, SepalWidget):
         Return:
             (bool): check if the value is initialized
         """
-        if not msg:
-            msg = "The value has not been initialized"
-        init = True
+        msg = msg or ms.utils.check_input.error
 
-        # check the collection type that are the only one supporting the len method
-        try:
-            init = False if len(input_) == 0 else init
-        except Exception:
-            init = False if input_ is None else init
-
-        if init is False:
-            self.add_msg(msg, "error")
-
-        return init
+        return su.check_input(input_, msg)
 
 
 class StateBar(v.SystemBar):


Test patch:
diff --git a/tests/test_Alert.py b/tests/test_Alert.py
index 6116cbc7..d125b74e 100644
--- a/tests/test_Alert.py
+++ b/tests/test_Alert.py
@@ -111,28 +111,22 @@ class TestAlert:
 
         alert = sw.Alert()
 
-        var_test = None
-        res = alert.check_input(var_test)
-        assert res is False
-        assert alert.viz is True
-        assert alert.children[0].children[0] == "The value has not been initialized"
+        with pytest.raises(ValueError, match="The value has not been initialized"):
+            alert.check_input(None)
 
-        res = alert.check_input(var_test, "toto")
-        assert alert.children[0].children[0] == "toto"
+        with pytest.raises(ValueError, match="toto"):
+            alert.check_input(None, "toto")
 
-        var_test = 1
-        res = alert.check_input(var_test)
+        res = alert.check_input(1)
         assert res is True
 
         # test lists
-        var_test = [range(2)]
-        res = alert.check_input(var_test)
+        res = alert.check_input([range(2)])
         assert res is True
 
         # test empty list
-        var_test = []
-        res = alert.check_input(var_test)
-        assert res is False
+        with pytest.raises(ValueError):
+            alert.check_input([])
 
         return
 
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 59bda7b8..9f6742cd 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -391,3 +391,24 @@ class TestUtils:
             su.geojson_to_ee(dict_)
 
         return
+
+    def test_check_input(self):
+
+        with pytest.raises(ValueError, match="The value has not been initialized"):
+            su.check_input(None)
+
+        with pytest.raises(ValueError, match="toto"):
+            su.check_input(None, "toto")
+
+        res = su.check_input(1)
+        assert res is True
+
+        # test lists
+        res = su.check_input([range(2)])
+        assert res is True
+
+        # test empty list
+        with pytest.raises(ValueError):
+            su.check_input([])
+
+        return