Repository: wright-group/attune

Issue:
Settables robustness
Setables are not really anything other than a string for their name at this point... which raises the question: do they need to exist at all

Currently if there is a tune that is not an arrangement or setable `__call__` fails.

I think it should fall back to assuming it is a setable if it is not an arrangement

We made setables objects such that they can have null positions (for arrangements which do not use them), but that has not been implemented.

 I propose that we make setables completely optional, and basically unused until we have such null behavior.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
new file mode 100644
index 0000000..28c3b65
--- /dev/null
+++ b/CHANGELOG.md
@@ -0,0 +1,17 @@
+# Changelog
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/).
+
+## [Unreleased]
+
+### Fixed
+- Writing NDarrays to `instrument.json` files
+
+## [0.4.0]
+
+### Added
+- initial release after a major rewrite
+
+[Unreleased]: https://github.com/wright-group/attune/compare/0.4.0...master
+[0.4.0]: https://github.com/wright-group/attune/releases/tag/0.4.0
diff --git a/attune/_arrangement.py b/attune/_arrangement.py
index bab449f..d4d867e 100644
--- a/attune/_arrangement.py
+++ b/attune/_arrangement.py
@@ -31,8 +31,6 @@ class Arrangement:
             k: Tune(**v) if isinstance(v, dict) else v for k, v in tunes.items()
         }
         self._ind_units: str = "nm"
-        self._ind_max: float = min([t.ind_max for t in self._tunes.values()])
-        self._ind_min: float = max([t.ind_min for t in self._tunes.values()])
 
     def __repr__(self):
         return f"Arrangement({repr(self.name)}, {repr(self.tunes)})"
@@ -81,11 +79,11 @@ class Arrangement:
 
     @property
     def ind_max(self):
-        return self._ind_max
+        return min([t.ind_max for t in self._tunes.values()])
 
     @property
     def ind_min(self):
-        return self._ind_min
+        return max([t.ind_min for t in self._tunes.values()])
 
     @property
     def name(self):
diff --git a/attune/_instrument.py b/attune/_instrument.py
index a3260eb..39b8ae6 100644
--- a/attune/_instrument.py
+++ b/attune/_instrument.py
@@ -15,7 +15,7 @@ class Instrument(object):
     def __init__(
         self,
         arrangements: Dict["str", Union[Arrangement, dict]],
-        setables: Dict["str", Union[Setable, dict]],
+        setables: Dict["str", Optional[Union[Setable, dict]]] = None,
         *,
         name: Optional[str] = None,
         transition: Optional[Union[Transition, dict]] = None,
@@ -25,6 +25,8 @@ class Instrument(object):
         self._arrangements: Dict["str", Arrangement] = {
             k: Arrangement(**v) if isinstance(v, dict) else v for k, v in arrangements.items()
         }
+        if setables is None:
+            setables = {}
         self._setables: Dict["str", Setable] = {
             k: Setable(**v) if isinstance(v, dict) else v for k, v in setables.items()
         }
@@ -77,20 +79,20 @@ class Instrument(object):
             raise ValueError("There are multiple valid arrangements! You must specify one.")
         # call arrangement
         setable_positions = {}
+        setables = self._setables.copy()
         todo = [(ind_value, tune) for tune in arrangement.tunes.items()]
         while todo:
             v, t = todo.pop(0)
             tune_name, tune = t
-            if tune_name in self._setables:
-                assert tune_name not in setable_positions
-                setable_positions[tune_name] = tune(v)
-            elif tune_name in self._arrangements:
+            if tune_name in self._arrangements:
                 new = [
                     (tune(v), subtune) for subtune in self._arrangements[tune_name].tunes.items()
                 ]
                 todo += new
             else:
-                raise ValueError(f"Unrecognized name {tune_name}")
+                assert tune_name not in setable_positions
+                setable_positions[tune_name] = tune(v)
+                setables[tune_name] = Setable(tune_name)
         # finish
         note = Note(
             setables=self._setables,
@@ -133,4 +135,11 @@ class Instrument(object):
 
     def save(self, file):
         """Save the JSON representation into an open file."""
-        json.dump(self.as_dict(), file)
+
+        class NdarrayEncoder(json.JSONEncoder):
+            def default(self, obj):
+                if hasattr(obj, "tolist"):
+                    return obj.tolist()
+                return json.JSONEncoder.default(self, obj)
+
+        json.dump(self.as_dict(), file, cls=NdarrayEncoder)
diff --git a/attune/_tune.py b/attune/_tune.py
index 259e389..9e9e50c 100644
--- a/attune/_tune.py
+++ b/attune/_tune.py
@@ -31,8 +31,6 @@ class Tune:
         dependent = np.asarray(dependent)
         assert independent.size == dependent.size
         assert independent.ndim == dependent.ndim == 1
-        self._ind_max = max(independent)
-        self._ind_min = min(independent)
         self._ind_units = "nm"
         self._dep_units = dep_units
         self._interp = scipy.interpolate.interp1d(independent, dependent, fill_value="extrapolate")
@@ -79,11 +77,11 @@ class Tune:
 
     @property
     def ind_max(self):
-        return self._ind_max
+        return self.independent.max()
 
     @property
     def ind_min(self):
-        return self._ind_min
+        return self.independent.min()
 
     @property
     def ind_units(self):


Test patch:
diff --git a/tests/instrument/test_call.py b/tests/instrument/test_call.py
index 9ae5426..ba2c911 100644
--- a/tests/instrument/test_call.py
+++ b/tests/instrument/test_call.py
@@ -30,3 +30,12 @@ def test_nested():
     second = attune.Arrangement("second", {"first": tune1})
     inst = attune.Instrument({"first": first, "second": second}, {"tune": attune.Setable("tune")})
     assert math.isclose(inst(0.75, "second")["tune"], 0.25)
+
+
+def test_implicit_setable():
+    tune = attune.Tune([0, 1], [0, 1])
+    tune1 = attune.Tune([0.5, 1.5], [0, 1])
+    first = attune.Arrangement("first", {"tune": tune})
+    second = attune.Arrangement("second", {"first": tune1})
+    inst = attune.Instrument({"first": first, "second": second})
+    assert math.isclose(inst(0.75, "second")["tune"], 0.25)
diff --git a/tests/map.py b/tests/map.py
index d3d3887..351ca18 100644
--- a/tests/map.py
+++ b/tests/map.py
@@ -16,6 +16,8 @@ def test_map_ind_points():
         inst1["test_map"]["test"](test_points), inst0["test_map"]["test"](test_points)
     )
     assert len(inst1["test_map"]["test"]) == 25
+    assert inst1["test_map"].ind_min == 1310
+    assert inst1["test_map"].ind_max == 1450
 
 
 def test_map_ind_limits():
diff --git a/tests/store/test_store.py b/tests/store/test_store.py
index 97c32e7..8109c30 100644
--- a/tests/store/test_store.py
+++ b/tests/store/test_store.py
@@ -4,6 +4,7 @@ import shutil
 import tempfile
 
 import attune
+import numpy as np
 import pytest
 
 here = pathlib.Path(__file__).parent
@@ -46,3 +47,12 @@ def test_load_store():
     instr = attune.load("test")
     with pytest.warns(UserWarning, match="Attempted to store instrument equivalent"):
         attune.store(instr)
+
+
+@temp_store
+def test_store_ndarray():
+    instr = attune.load("test")
+    instr = attune.map_ind_points(instr, "arr", "tune", np.linspace(0.25, 1, 5))
+    # Would raise here because it is trying to serialize the ndarray in metadata
+    # prior to bug fix
+    attune.store(instr)