Repository: wright-group/WrightTools

Issue:
api for "forcing" unit conversion of underlying variables
currently, `data.convert` and `axis.convert` simply change the units that are returned when slicing into the axis object---the variables themselves remain untouched---I like this behavior and want to keep it

in addition to the above, users should be able to convert the variables directly

step one: add `convert` method to `Variable` class

step two: add `convert_variables=False` kwarg to `data.convert`, `axis.convert`, which toggles propagation to dependent variables

we should think carefully, because variables can appear in multiple axes

Discussion:
pep8speaks: Hello @untzag! Thanks for submitting the PR.

 - In the file [`WrightTools/_dataset.py`](https://github.com/wright-group/WrightTools/blob/b2240c89d941647b42212c46b3463c1bc55b9d71/WrightTools/_dataset.py), following are the PEP8 issues :

> [Line 217:9](https://github.com/wright-group/WrightTools/blob/b2240c89d941647b42212c46b3463c1bc55b9d71/WrightTools/_dataset.py#L217): [E306](https://duckduckgo.com/?q=pep8%20E306) expected 1 blank line before a nested definition, found 0











Patch:
diff --git a/WrightTools/_dataset.py b/WrightTools/_dataset.py
index 0331efe..9f46ef3 100644
--- a/WrightTools/_dataset.py
+++ b/WrightTools/_dataset.py
@@ -6,13 +6,14 @@
 
 import posixpath
 import collections
-from concurrent.futures import ThreadPoolExecutor
 
 import numpy as np
 
 import h5py
 
+from . import exceptions as wt_exceptions
 from . import kit as wt_kit
+from . import units as wt_units
 
 
 # --- class ---------------------------------------------------------------------------------------
@@ -196,6 +197,27 @@ class Dataset(h5py.Dataset):
 
         self.chunkwise(f, min=min, max=max, replace=replace)
 
+    def convert(self, destination_units):
+        """Convert units.
+
+        Parameters
+        ----------
+        destination_units : string (optional)
+            Units to convert into.
+        """
+        if not wt_units.is_valid_conversion(self.units, destination_units):
+            kind = wt_units.kind(self.units)
+            valid = list(wt_units.dicts[kind].keys())
+            raise wt_exceptions.UnitsError(valid, destination_units)
+        if self.units is None:
+            return
+
+        def f(dataset, s, destination_units):
+            dataset[s] = wt_units.converter(dataset[s], dataset.units, destination_units)
+
+        self.chunkwise(f, destination_units=destination_units)
+        self.units = destination_units
+
     def log(self, base=np.e, floor=None):
         """Take the log of the entire dataset.
 
diff --git a/WrightTools/data/_axis.py b/WrightTools/data/_axis.py
index 11132b0..1485046 100644
--- a/WrightTools/data/_axis.py
+++ b/WrightTools/data/_axis.py
@@ -79,20 +79,10 @@ class Axis(object):
 
     @property
     def label(self):
-        label_seed = [v.label for v in self.variables]
-        symbol_type = wt_units.get_default_symbol_type(self.units)
-        label = r'$\mathsf{'
-        for part in label_seed:
-            if self.units_kind is not None:
-                units_dictionary = getattr(wt_units, self.units_kind)
-                label += getattr(wt_units, symbol_type)[self.units]
-                if part is not '':
-                    label += r'_{' + str(part) + r'}'
-            else:
-                label += self.name.replace('_', '\,\,')
-            # TODO: handle all operators
-            label += r'='
-        label = label[:-1]  # remove the last equals sign
+        symbol = wt_units.get_symbol(self.units)
+        label = r'$\mathsf{' + self.expression
+        for v in self.variables:
+            label = label.replace(v.natural_name, '%s_{%s}' % (symbol, v.label))
         if self.units_kind:
             units_dictionary = getattr(wt_units, self.units_kind)
             label += r'\,'
@@ -139,9 +129,7 @@ class Axis(object):
     @property
     def units_kind(self):
         """Units kind."""
-        for dic in wt_units.unit_dicts:
-            if self.units in dic.keys():
-                return dic['kind']
+        return wt_units.kind(self.units)
 
     @property
     def variables(self):
@@ -156,21 +144,23 @@ class Axis(object):
         finally:
             return self._variables
 
-    def convert(self, destination_units):
+    def convert(self, destination_units, *, convert_variables=False):
         """Convert axis to destination_units.
 
         Parameters
         ----------
         destination_units : string
             Destination units.
+        convert_variables : boolean (optional)
+            Toggle conversion of stored arrays. Default is False.
         """
-        destination_units_kind = None
-        for dic in wt_units.unit_dicts:
-            if destination_units in dic.keys():
-                destination_units_kind = dic['kind']
-                break
-        if not self.units_kind == destination_units_kind:
-            raise wt_exceptions.UnitsError(self.units_kind, destination_units_kind)
+        if not wt_units.is_valid_conversion(self.units, destination_units):
+            kind = wt_units.kind(self.units)
+            valid = list(wt_units.dicts[kind].keys())
+            raise wt_exceptions.UnitsError(valid, destination_units)
+        if convert_variables:
+            for v in self.variables:
+                v.convert(destination_units)
         self.units = destination_units
 
     def max(self):
diff --git a/WrightTools/data/_data.py b/WrightTools/data/_data.py
index fba69ab..09e060d 100644
--- a/WrightTools/data/_data.py
+++ b/WrightTools/data/_data.py
@@ -313,9 +313,19 @@ class Data(Group):
             idx = tuple(idx)
             data = out.create_data(name='chop%03i' % i)
             for v in self.variables:
-                data.create_variable(name=v.natural_name, values=v[idx], units=v.units)
+                kwargs = {}
+                kwargs['name'] = v.natural_name
+                kwargs['values'] = v[idx]
+                kwargs['units'] = v.units
+                kwargs['label'] = v.label
+                data.create_variable(**kwargs)
             for c in self.channels:
-                data.create_channel(name=c.natural_name, values=c[idx], units=c.units)
+                kwargs = {}
+                kwargs['name'] = c.natural_name
+                kwargs['values'] = c[idx]
+                kwargs['units'] = c.units
+                kwargs['label'] = c.label
+                data.create_channel(**kwargs)
             data.transform([a.expression for a in kept_axes if a.expression not in at.keys()])
             i += 1
         out.flush()
@@ -380,13 +390,15 @@ class Data(Group):
         self._axes.pop(axis_index)
         self._update_natural_namespace()
 
-    def convert(self, destination_units, verbose=True):
-        """Convert all compatable constants and axes to given units.
+    def convert(self, destination_units, *, convert_variables=False, verbose=True):
+        """Convert all compatable axes to given units.
 
         Parameters
         ----------
         destination_units : str
             Destination units.
+        convert_variables : boolean (optional)
+            Toggle conversion of stored arrays. Default is False
         verbose : bool (optional)
             Toggle talkback. Default is True.
 
@@ -397,13 +409,11 @@ class Data(Group):
             axis object in data.axes or data.constants.
         """
         # get kind of units
-        for dic in wt_units.unit_dicts:
-            if destination_units in dic.keys():
-                units_kind = dic['kind']
+        units_kind = wt_units.kind(destination_units)
         # apply to all compatible axes
         for axis in self.axes + self.constants:
             if axis.units_kind == units_kind:
-                axis.convert(destination_units)
+                axis.convert(destination_units, convert_variables=convert_variables)
                 if verbose:
                     print('axis', axis.expression, 'converted')
 
diff --git a/WrightTools/kit/_array.py b/WrightTools/kit/_array.py
index c74c4b9..b7cd2d1 100644
--- a/WrightTools/kit/_array.py
+++ b/WrightTools/kit/_array.py
@@ -270,12 +270,18 @@ def valid_index(index, shape):
     tuple
         Valid index.
     """
+    # append slices to index
+    index = list(index)
+    while len(index) < len(shape):
+        index.append(slice(None))
+    # fill out, in reverse
     out = []
     for i, s in zip(index[::-1], shape[::-1]):
         if s == 1:
-            out.append(0)
-        elif isinstance(i, slice):
-            out.append(i)
+            if isinstance(i, slice):
+                out.append(slice(None))
+            else:
+                out.append(0)
         else:
-            out.append(min(s - 1, i))
+            out.append(i)
     return tuple(out[::-1])
diff --git a/WrightTools/units.py b/WrightTools/units.py
index 131f667..76383da 100644
--- a/WrightTools/units.py
+++ b/WrightTools/units.py
@@ -4,34 +4,31 @@
 # --- import --------------------------------------------------------------------------------------
 
 
-from __future__ import absolute_import, division, print_function, unicode_literals
+import collections
 
 import numpy as np
 import warnings
 
 
-# --- units ---------------------------------------------------------------------------------------
+# --- define --------------------------------------------------------------------------------------
 
 
 # units are stored in dictionaries of like kind. format:
 #     unit : to native, from native, units_symbol, units_label
 
 # angle units (native: rad)
-angle = {'kind': 'angle',
-         'rad': ['x', 'x', r'rad'],
+angle = {'rad': ['x', 'x', r'rad'],
          'deg': ['x/57.2958', '57.2958*x', r'deg']}
 
 # delay units (native: fs)
 fs_per_mm = 3336.
-delay = {'kind': 'delay',
-         'fs': ['x', 'x', r'fs'],
+delay = {'fs': ['x', 'x', r'fs'],
          'ps': ['x*1e3', 'x/1e3', r'ps'],
          'ns': ['x*1e6', 'x/1e6', r'ns'],
          'mm_delay': ['x*2*fs_per_mm', 'x/(2*fs_per_mm)', r'mm']}
 
 # energy units (native: nm)
-energy = {'kind': 'energy',
-          'nm': ['x', 'x', r'nm'],
+energy = {'nm': ['x', 'x', r'nm'],
           'wn': ['1e7/x', '1e7/x', r'cm^{-1}'],
           'eV': ['1240./x', '1240./x', r'eV'],
           'meV': ['1240000./x', '1240000./x', r'meV'],
@@ -40,30 +37,24 @@ energy = {'kind': 'energy',
           'GHz': ['2.99792458e8/x', '2.99792458e8/x', r'GHz']}
 
 # fluence units (native: uJ per sq. cm)
-fluence = {'kind': 'fluence',
-           'uJ per sq. cm': ['x', 'x', r'\frac{\mu J}{cm^{2}}']}
+fluence = {'uJ per sq. cm': ['x', 'x', r'\frac{\mu J}{cm^{2}}']}
 
 # optical density units (native: od)
-od = {'kind': 'od',
-      'mOD': ['1e3*x', 'x/1e3', r'mOD'],
+od = {'mOD': ['1e3*x', 'x/1e3', r'mOD'],
       'OD': ['x', 'x', r'OD']}
 
 # position units (native: mm)
-position = {'kind': 'position',
-            # can't have same name as energy nm
-            'nm_p': ['x/1e6', '1e6/x', r'nm'],
+position = {'nm_p': ['x/1e6', '1e6/x', r'nm'],
             'um': ['x/1000.', '1000/x.', r'um'],
             'mm': ['x', 'x', r'mm'],
             'cm': ['10.*x', 'x/10.', r'cm'],
             'in': ['x*0.039370', '0.039370*x', r'in']}
 
 # pulse width units (native: FWHM)
-pulse_width = {'kind': 'pulse_width',
-               'FWHM': ['x', 'x', r'FWHM']}
+pulse_width = {'FWHM': ['x', 'x', r'FWHM']}
 
 # time units (native: s)
-time = {'kind': 'time',
-        'fs_t': ['x/1e15', 'x*1e15', r'fs'],
+time = {'fs_t': ['x/1e15', 'x*1e15', r'fs'],
         'ps_t': ['x/1e12', 'x*1e12', r'ps'],
         'ns_t': ['x/1e9', 'x*1e9', r'ns'],
         'us_t': ['x/1e6', 'x*1e6', r'us'],
@@ -73,7 +64,18 @@ time = {'kind': 'time',
         'h_t': ['x*3600.', 'x/3600.', r'h'],
         'd_t': ['x*86400.', 'x/86400.', r'd']}
 
-unit_dicts = [angle, delay, energy, time, position, pulse_width, fluence, od]
+dicts = collections.OrderedDict()
+dicts['angle'] = angle
+dicts['delay'] = delay
+dicts['energy'] = energy
+dicts['time'] = time
+dicts['position'] = position
+dicts['pulse_width'] = pulse_width
+dicts['fluence'] = fluence
+dicts['od'] = od
+
+
+# --- functions -----------------------------------------------------------------------------------
 
 
 def converter(val, current_unit, destination_unit):
@@ -94,7 +96,7 @@ def converter(val, current_unit, destination_unit):
         Converted value.
     """
     x = val
-    for dic in unit_dicts:
+    for dic in dicts.values():
         if current_unit in dic.keys() and destination_unit in dic.keys():
             try:
                 native = eval(dic[current_unit][0])
@@ -111,86 +113,71 @@ def converter(val, current_unit, destination_unit):
         pass
     else:
         warnings.warn('conversion {0} to {1} not valid: returning input'.format(
-                       current_unit, destination_unit))
+                      current_unit, destination_unit))
     return val
 
 
-def kind(units):
-    """Find the kind of given units.
+def get_symbol(units):
+    """Get default symbol type.
 
     Parameters
     ----------
-    units : string
-        The units of interest
+    units_str : string
+        Units.
 
     Returns
     -------
     string
-        The kind of the given units. If no match is found, returns None.
+        LaTeX formatted symbol.
     """
-    for d in unit_dicts:
-        if units in d.keys():
-            return str(d['kind'])
-
-
-# --- symbol --------------------------------------------------------------------------------------
-
-
-class SymbolDict(dict):
-    """Subclass dictionary to get at __missing__ method."""
-
-    def __missing__(self, key):
-        """Define what happens when key is missing."""
-        return self['default']
-
-
-# color
-color_symbols = SymbolDict()
-color_symbols['default'] = r'E'
-color_symbols['nm'] = r'\lambda'
-color_symbols['wn'] = r'\bar\nu'
-color_symbols['eV'] = r'\hslash\omega'
-color_symbols['Hz'] = r'f'
-color_symbols['THz'] = r'f'
-color_symbols['GHz'] = r'f'
+    if kind(units) == 'energy':
+        d = {}
+        d['nm'] = r'\lambda'
+        d['wn'] = r'\bar\nu'
+        d['eV'] = r'\hslash\omega'
+        d['Hz'] = r'f'
+        d['THz'] = r'f'
+        d['GHz'] = r'f'
+        return d.get(units, 'E')
+    elif kind(units) == 'delay':
+        return r'\tau'
+    elif kind(units) == 'fluence':
+        return r'\mathcal{F}'
+    elif kind(units) == 'pulse_width':
+        return r'\sigma'
+    else:
+        return kind(units)
 
-# delay
-delay_symbols = SymbolDict()
-delay_symbols['default'] = r'\tau'
 
-# fluence
-fluence_symbols = SymbolDict()
-fluence_symbols['default'] = r'\mathcal{F}'
+def get_valid_conversions(units):
+    valid = list(dicts[kind(units)])
+    valid.remove(units)
+    return tuple(valid)
 
-# pulse width
-pulse_width_symbols = SymbolDict()
-pulse_width_symbols['default'] = r'\sigma'
 
-# catch all
-none_symbols = SymbolDict()
-none_symbols['default'] = ''
+def is_valid_conversion(a, b):
+    for dic in dicts.values():
+        if a in dic.keys() and b in dic.keys():
+            return True
+    if a is None and b is None:
+        return True
+    else:
+        return False
 
 
-def get_default_symbol_type(units_str):
-    """Get default symbol type.
+def kind(units):
+    """Find the kind of given units.
 
     Parameters
     ----------
-    units_str : string
-        Units.
+    units : string
+        The units of interest
 
     Returns
     -------
     string
-        Symbol dictionary name.
+        The kind of the given units. If no match is found, returns None.
     """
-    if units_str in ['nm', 'wn', 'eV']:
-        return 'color_symbols'
-    elif units_str in ['fs', 'ps', 'ns']:
-        return 'delay_symbols'
-    elif units_str in ['uJ per sq. cm']:
-        return 'fluence_symbols'
-    elif units_str in ['FWHM']:
-        return 'pulse_width_symbols'
-    else:
-        return 'none_symbols'
+    for k, v in dicts.items():
+        if units in v.keys():
+            return k


Test patch:
diff --git a/tests/data/axis/convert_axis.py b/tests/data/axis/convert_axis.py
new file mode 100644
index 0000000..4e1408b
--- /dev/null
+++ b/tests/data/axis/convert_axis.py
@@ -0,0 +1,68 @@
+"""Test axis unit conversion."""
+
+
+# --- import --------------------------------------------------------------------------------------
+
+
+import numpy as np
+
+import WrightTools as wt
+from WrightTools import datasets
+
+
+# --- define --------------------------------------------------------------------------------------
+
+
+def test_convert_variables():
+    p = datasets.KENT.LDS821_TRSF
+    ignore = ['d1', 'd2', 'wm']
+    data = wt.data.from_KENT(p, ignore=ignore)
+    data.w2.convert('meV', convert_variables=True)
+    assert data.w2.units == 'meV'
+    assert data['w2'].units == 'meV'
+    data.close()
+
+
+def test_exception():
+    p = datasets.PyCMDS.w1_000
+    data = wt.data.from_PyCMDS(p)
+    try:
+        data.w1.convert('fs')
+    except wt.exceptions.UnitsError:
+        assert True
+    else:
+        assert False
+    assert data.w1.units == 'nm'
+    assert data['w1'].units == 'nm'
+    data.close()
+
+
+def test_w1_wa():
+    p = datasets.PyCMDS.w1_wa_000
+    data = wt.data.from_PyCMDS(p)
+    assert data.wa.units == 'nm'
+    data.wa.convert('eV')
+    assert data.wa.units == 'eV'
+    assert np.isclose(data.wa.max(), 1.5802564757220569)
+    assert np.isclose(data.wa.min(), 0.6726385958618104)
+    assert data['wa'].units == 'nm'
+    data.close()
+
+
+def test_wigner():
+    p = datasets.COLORS.v2p2_WL_wigner
+    data = wt.data.from_COLORS(p)
+    data.d1.convert('ns')
+    assert data.d1.units == 'ns'
+    assert data['d1'].units == 'fs'
+    data.close()
+
+
+# --- run -----------------------------------------------------------------------------------------
+
+
+if __name__ == '__main__':
+    test_convert_variables()
+    test_exception()
+    test_w1_wa()
+    test_wigner()
diff --git a/tests/data/chop.py b/tests/data/chop.py
index 1b59ed1..0f2f37e 100755
--- a/tests/data/chop.py
+++ b/tests/data/chop.py
@@ -20,6 +20,8 @@ def test_2D_to_1D():
     for d in chop.values():
         assert d.w2.size == 81
         assert d.axis_expressions == ('w2',)
+        for k in data.variable_names:
+            assert d[k].label == data[k].label
     data.close()
     chop.close()
 
@@ -85,4 +87,5 @@ def test_parent():
 
 
 if __name__ == "__main__":
+    test_2D_to_1D()
     test_3D_to_1D()
diff --git a/tests/data/convert_data.py b/tests/data/convert_data.py
new file mode 100644
index 0000000..e232390
--- /dev/null
+++ b/tests/data/convert_data.py
@@ -0,0 +1,57 @@
+"""Test data unit conversion."""
+
+
+# --- import --------------------------------------------------------------------------------------
+
+
+import numpy as np
+
+import WrightTools as wt
+from WrightTools import datasets
+
+
+# --- define --------------------------------------------------------------------------------------
+
+
+def test_convert_variables():
+    p = datasets.KENT.LDS821_TRSF
+    ignore = ['d1', 'd2', 'wm']
+    data = wt.data.from_KENT(p, ignore=ignore)
+    data.convert('meV', convert_variables=True)
+    assert data.w1.units == 'meV'
+    assert data.w2.units == 'meV'
+    assert data['w2'].units == 'meV'
+    assert data['w2'].units == 'meV'
+    data.close()
+
+
+def test_w1_wa():
+    p = datasets.PyCMDS.w1_wa_000
+    data = wt.data.from_PyCMDS(p)
+    assert data.wa.units == 'nm'
+    data.convert('eV')
+    assert data.wa.units == 'eV'
+    assert np.isclose(data.wa.max(), 1.5802564757220569)
+    assert np.isclose(data.wa.min(), 0.6726385958618104)
+    assert data['wa'].units == 'nm'
+    data.close()
+
+
+def test_wigner():
+    p = datasets.COLORS.v2p2_WL_wigner
+    data = wt.data.from_COLORS(p)
+    data.convert('ns')
+    assert data.d1.units == 'ns'
+    assert data['d1'].units == 'fs'
+    assert data.wm.units == 'nm'
+    assert data['wm'].units == 'nm'
+    data.close()
+
+
+# --- run -----------------------------------------------------------------------------------------
+
+
+if __name__ == '__main__':
+    test_convert_variables()
+    test_w1_wa()
+    test_wigner()
diff --git a/tests/dataset/convert_dataset.py b/tests/dataset/convert_dataset.py
new file mode 100644
index 0000000..6c82d5a
--- /dev/null
+++ b/tests/dataset/convert_dataset.py
@@ -0,0 +1,53 @@
+"""Test dataset unit conversion."""
+
+
+# --- import --------------------------------------------------------------------------------------
+
+
+import numpy as np
+
+import WrightTools as wt
+from WrightTools import datasets
+
+
+# --- define --------------------------------------------------------------------------------------
+
+
+def test_exception():
+    p = datasets.PyCMDS.w1_000
+    data = wt.data.from_PyCMDS(p)
+    try:
+        data['w1'].convert('fs')
+    except wt.exceptions.UnitsError:
+        assert True
+    else:
+        assert False
+    assert data['w1'].units == 'nm'
+    data.close()
+
+
+def test_w1_wa():
+    p = datasets.PyCMDS.w1_wa_000
+    data = wt.data.from_PyCMDS(p)
+    assert data['wa'].units == 'nm'
+    data['wa'].convert('eV')
+    assert np.isclose(data['wa'].max(), 1.5802564757220569)
+    assert np.isclose(data['wa'].min(), 0.6726385958618104)
+    data.close()
+
+
+def test_wigner():
+    p = datasets.COLORS.v2p2_WL_wigner
+    data = wt.data.from_COLORS(p)
+    data['d1'].convert('ns')
+    assert data['d1'].units == 'ns'
+    data.close()
+
+
+# --- run -----------------------------------------------------------------------------------------
+
+
+if __name__ == '__main__':
+    test_exception()
+    test_w1_wa()
+    test_wigner()
diff --git a/tests/kit/valid_index.py b/tests/kit/valid_index.py
new file mode 100644
index 0000000..16a0e53
--- /dev/null
+++ b/tests/kit/valid_index.py
@@ -0,0 +1,65 @@
+"""Test valid index function."""
+
+
+# --- import --------------------------------------------------------------------------------------
+
+
+import WrightTools as wt
+
+
+# --- test ----------------------------------------------------------------------------------------
+
+
+def test__1_5__7():
+    index = (1, 5)
+    shape = (7,)
+    assert wt.kit.valid_index(index, shape) == (5,)
+
+
+def test__4_2_12__1_25_1():
+    index = (4, 2, 12)
+    shape = (1, 25, 1)
+    assert wt.kit.valid_index(index, shape) == (0, 2, 0)
+
+
+def test__s__23():
+    index = (slice(None),)
+    shape = (23,)
+    assert wt.kit.valid_index(index, shape) == (slice(None),)
+
+
+def test__s__1_25():
+    index = (slice(None),)
+    shape = (1, 25,)
+    assert wt.kit.valid_index(index, shape) == (slice(None), slice(None))
+
+
+def test__ss_ss__1_25():
+    index = (slice(20, None, 1), slice(20, None, 1))
+    shape = (1, 25,)
+    assert wt.kit.valid_index(index, shape) == (slice(None), slice(20, None, 1))
+
+
+def test__s__13_25_99():
+    index = (slice(None),)
+    shape = (13, 25, 99)
+    assert wt.kit.valid_index(index, shape) == (slice(None), slice(None), slice(None))
+
+
+def test__s_s__51():
+    index = (slice(None), slice(None))
+    shape = (51,)
+    assert wt.kit.valid_index(index, shape) == (slice(None),)
+
+
+# --- run -----------------------------------------------------------------------------------------
+
+
+if __name__ == '__main__':
+    test__1_5__7()
+    test__4_2_12__1_25_1()
+    test__s__23()
+    test__s__1_25()
+    test__ss_ss__1_25()
+    test__s__13_25_99()
+    test__s_s__51()