Merge pull request #270 from DHI/fix/named-style

Fix loading named styles

.github/workflows/python-publish.yml

@@ -1,18 +1,17 @@
-# This workflows will upload a Python Package using Twine when a release is created
-# For more information see: https://help.github.com/en/actions/language-and-framework-guides/using-python-with-github-actions#publishing-to-package-registries
-
 name: Upload Python Package
 
 on:
-  workflow_dispatch:
-
   release:
     types: [created]
-
+  workflow_dispatch:
+
 jobs:
   deploy:
 
     runs-on: ubuntu-latest
+    permissions:
+      # IMPORTANT: this permission is mandatory for trusted publishing
+      id-token: write
 
     steps:
     - uses: actions/checkout@v3
@@ -28,10 +27,7 @@ jobs:
       run: pip install .[test]
     - name: Test
       run: pytest
-    - name: Build and publish
-      env:
-        TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }}
-        TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }}
-      run: |
-        python -m build
-        twine upload dist/*
+    - name: Build  
+      run: python -m build
+    - name: Publish package distributions to PyPI
+      uses: pypa/gh-action-pypi-publish@release/v1

modelskill/settings.py

@@ -112,17 +112,20 @@ def _set_option(*args, **kwargs) -> None:
 
     if len(args) == 1 and isinstance(args[0], dict):
         kwargs.update(args[0])
-    else:
-        nargs = len(args)
-        if not nargs or nargs % 2 != 0:
-            raise ValueError(
-                f"Must provide an even number of non-keyword arguments, Input was args={args}, kwargs={kwargs}"
-            )
+
+    if len(args) % 2 == 0:
+        keys = args[::2]
+        values = args[1::2]
+        kwargs.update(dict(zip(keys, values)))
+
+    if len(args) > 1 and len(args) % 2 != 0:
+        raise ValueError("Must provide a value for each key, i.e. even number of args")
+
 
     # default to false
     kwargs.pop("silent", False)
 
-    for k, v in zip(args[::2], args[1::2]):
+    for k, v in kwargs.items():
         key = _get_single_key(k)  # , silent)
 
         o = _get_registered_option(key)

tests/test_settings.py

@@ -68,6 +68,10 @@ def test_options_set_options():
         modelskill.set_option("plot.scatter.points.size", "200")
         modelskill.set_option("plot.scatter.points.size", -1)
 
+    # TODO extract into common teardown, this is needed since options is global state
+    # reset all options
+    modelskill.reset_option("all")
+
 
 def test_options_set_invalid_metric_raises_error():
     o = modelskill.options
@@ -83,6 +87,9 @@ def test_options_set_invalid_metric_raises_error():
     with pytest.raises(ValueError, match="invalid_metric"):
         o.metrics.list = ["bias", "invalid_metric"]
 
+    # reset all options
+    modelskill.reset_option("all")
+
 
 def test_options_reset_options():
     o = modelskill.options
@@ -106,15 +113,43 @@ def test_options_register_option():
 
 
 def test_load_style_mood():
+    # defaults
+    assert len(modelskill.get_option("metrics.list")) == 7
+    assert modelskill.get_option("plot.scatter.points.size") == 20
+
+    # mood.yml
+    # -------------------------------
+    # plot.scatter.points.size: 10.0
+    # plot.scatter.oneone_line.color: darkorange
     settings.load_style(name="MOOD")
-    assert True
+    assert modelskill.get_option("plot.scatter.oneone_line.color") == "darkorange"
+    assert modelskill.get_option("plot.scatter.points.size") == 10.0
+
+    # metrics are not affected
+    assert len(modelskill.get_option("metrics.list")) == 7
+
+    # reset all options
+    modelskill.reset_option("all")
 
 
 def test_load_style_is_case_insensitive():
     settings.load_style(name="mOoD")
-    assert True
+    assert modelskill.get_option("plot.scatter.oneone_line.color") == "darkorange"
+
+    # reset all options
+    modelskill.reset_option("all")
 
 
 def test_unknown_style_raises_error():
     with pytest.raises(KeyError, match="food"):
         settings.load_style(name="food")
+
+
+def test_setting_options_need_even_number_of_args():
+    with pytest.raises(ValueError, match="even number"):
+        settings.set_option(
+            "plot.scatter.points.size", 10, "plot.scatter.oneone_line.color"
+        )
+
+    # reset all options
+    modelskill.reset_option("all")