Repository: Quasars/orange-spectroscopy (language: python)

Issue:
Multifile different size bug
If there are multiple files read with multifile and they are not the same length / number of points, the widget crashes and doesn't give a meaningful Error message.


Discussion / hints:
@borondics, this works for me or at least the files I tested. Could you be more specific? Or perhaps email me the files?

Also, does this happen in the dev install, the .dmg or both? Thanks!

Fix patch:
diff --git a/orangecontrib/spectroscopy/widgets/owmultifile.py b/orangecontrib/spectroscopy/widgets/owmultifile.py
index e3a85190c..8beecd58f 100644
--- a/orangecontrib/spectroscopy/widgets/owmultifile.py
+++ b/orangecontrib/spectroscopy/widgets/owmultifile.py
@@ -1,7 +1,8 @@
+import math
 import os
 from functools import reduce
 from itertools import chain, count, repeat
-from collections import Counter, namedtuple
+from collections import Counter, namedtuple, defaultdict
 from typing import List
 
 import numpy as np
@@ -41,6 +42,49 @@ def numpy_union_keep_order(A, B):
     return np.concatenate((A, to_add))
 
 
+def decimals_neeeded_for_unique_str(l):
+    min_diff = min(np.abs(np.diff(sorted(l))))
+    log_diff = math.log(min_diff, 10)
+    # We need to avoid possible degenerate cases like (1.5, 2.5):
+    # the log of the difference (1.0) is 0, but rounded to zero
+    # decimals we would get (2, 2) due to floating point rounding behavior.
+    # Therefore, slightly increase the number of needed decimals.
+    # The number was selected such that (1.5, 2.5) returned 1 decimal,
+    # and (1.49, 2.51) returned 0.
+    log_diff = log_diff - 0.008
+    decimals = max(0, math.ceil(-log_diff))
+    return decimals
+
+
+def wns_to_unique_str(l):
+    """ Convert a list on wns to a list of unique strings.
+
+    Use 6 decimal places by default. If that is not sufficient,
+    increase precision as needed for wns in conflict.
+    """
+
+    # 6 used to be the default: "%f" means "%0.6f"
+    default_decimals = 6
+    default_format = "%0." + str(default_decimals) + "f"
+
+    same = defaultdict(list)
+    for wn in l:
+        same[default_format % wn].append(wn)
+
+    decimals = {}
+    for n in same:
+        if len(same[n]) == 1:
+            decimals[n] = default_decimals
+        else:
+            # add one decimal place because without it the error could be too big
+            # for example, (1.49, 2.51) would round to 1 and 3, with an added decimal
+            # it will show (1.5, 2.5). Max error before was 0.5*last_decimal_place,
+            # which seems too big.
+            decimals[n] = decimals_neeeded_for_unique_str(same[n]) + 1
+
+    return [("%0." + str(decimals[default_format % wn]) + "f") % wn for wn in l]
+
+
 def concatenate_data(tables, filenames, label):
     if not tables:
         return None
@@ -53,7 +97,8 @@ def concatenate_data(tables, filenames, label):
            if hasattr(t, "special_spectral_data")]
     xs = reduce(numpy_union_keep_order, xss, np.array([]))
     if len(xs):
-        attrs = [ContinuousVariable("%f" % f) for f in xs]
+        names = wns_to_unique_str(xs)
+        attrs = [ContinuousVariable(n) for n in names]
         spectral_specific_domains = [Domain(attrs, None, None)]
 
     domain = _merge_domains(spectral_specific_domains + [table.domain for table in tables])


Test patch:
diff --git a/orangecontrib/spectroscopy/tests/test_owmultifile.py b/orangecontrib/spectroscopy/tests/test_owmultifile.py
index 93f42be85..642d31d56 100644
--- a/orangecontrib/spectroscopy/tests/test_owmultifile.py
+++ b/orangecontrib/spectroscopy/tests/test_owmultifile.py
@@ -18,7 +18,8 @@
 from orangecontrib.spectroscopy.io import SPAReader
 from orangecontrib.spectroscopy.io.ascii import AsciiColReader
 from orangecontrib.spectroscopy.io.util import SpectralFileFormat
-from orangecontrib.spectroscopy.widgets.owmultifile import OWMultifile, numpy_union_keep_order
+from orangecontrib.spectroscopy.widgets.owmultifile import OWMultifile, numpy_union_keep_order, \
+    wns_to_unique_str, decimals_neeeded_for_unique_str
 
 
 class TestOWFilesAuxiliary(unittest.TestCase):
@@ -34,12 +35,42 @@ def test_numpy_union(self):
         A = np.array([])
         np.testing.assert_equal(numpy_union_keep_order(A, B), [5, 4, 6, 3])
 
+    def test_decimals_neeeded_for_unique_str(self):
+        fn = decimals_neeeded_for_unique_str
+        self.assertEqual(fn([1, 2]), 1)
+        self.assertEqual(fn([1.5, 2.5]), 1)
+        self.assertEqual(fn([1.49, 2.51]), 0)
+        self.assertEqual(fn([10, 20]), 0)
+        self.assertEqual(fn([10.0, 10.1]), 2)
+        self.assertEqual(fn([10.0, 10.2]), 1)
+        self.assertEqual(fn([10.00000001, 10.00000002]), 9)
+        self.assertEqual(fn([10.00000001, 10.000000015, 10.00000002]), 9)
+        self.assertEqual(fn([10.00000001, 10.000000014, 10.00000002]), 9)
+        self.assertEqual(fn([10.00000001, 10.000000013, 10.00000002]), 9)
+        self.assertEqual(fn([10.00000001, 10.000000012, 10.00000002]), 9)
+        self.assertEqual(fn([10.00000001, 10.000000011, 10.00000002]), 10)
+
+    def test_wns_to_unique_str(self):
+        names = wns_to_unique_str([1, 2, 2 + 1e-10, 3])
+        self.assertEqual(names, ['1.000000', '2.000000000000', '2.000000000100', '3.000000'])
+        names = wns_to_unique_str([1, 2, 2 + 0.99e-10, 3])
+        self.assertEqual(names, ['1.000000', '2.000000000000', '2.000000000099', '3.000000'])
+        names = wns_to_unique_str([4, 1, 2, 2 + 0.99e-10, 3])
+        self.assertEqual(names, ['4.000000', '1.000000', '2.000000000000', '2.000000000099', '3.000000'])
+        names = wns_to_unique_str([2+2e-10, 1, 2, 2 + 1e-10, 3])
+        self.assertEqual(names, ['2.000000000200', '1.000000', '2.000000000000', '2.000000000100', '3.000000'])
+
 
 class TestOWMultifile(WidgetTest):
 
     def setUp(self):
         self.widget = self.create_widget(OWMultifile)  # type: OWMultifile
 
+    def tearDown(self):
+        super().tearDown()
+        # clear FileFormat cache so the optional new classes are thrown out
+        FileFormat._ext_to_attr_if_attr2.cache_clear()
+
     def test_load_unload(self):
         # just to load the widget (it has no inputs)
         pass
@@ -199,8 +230,27 @@ def read_spectra(self):
             self.assertEqual(CountSPAReader.read_count, 0)
             self.assertEqual(CountSPAReader.read_spectra_count, 1)
             self.assertEqual(CountTabReader.read_count, 1)
-            # clear cache so the new classes are thrown out
+
+    def test_spectra_almost_same_wavenumbers(self):
+
+        class ReadImaginaryFile(SPAReader):
+            read_count = -1
+
+            def read_spectra(self):
+                type(self).read_count += 1
+                if type(self).read_count == 0:
+                    return np.array([1, 2]), np.array([[42, 41]]), None
+                return np.array([1, 2 + 1e-10]), np.array([[43, 44]]), None
+
+        with patch.object(FileFormat, "registry", {"SPAReader": ReadImaginaryFile}):
+            # clear LRU cache so that new classes get use
             FileFormat._ext_to_attr_if_attr2.cache_clear()
+            self.load_files("sample1.spa", "sample1.spa")
+            out = self.get_output("Data")
+            np.testing.assert_equal(out.X, [[42, 41, np.nan],
+                                            [43, np.nan, 44]])
+            self.assertEqual([a.name for a in out.domain.attributes],
+                             ['1.000000', '2.000000000000', '2.000000000100'])
 
     def test_report_on_empty(self):
         self.widget.send_report()