Repository: SpikeInterface/spikeinterface
Pull request #1529 (2023-04-13 07:58:13)

Add tests for json encoding of nested numpy scalars 
Temporary branch for debugging error #1525 and possible #1526.

Edit I: OK. I am waiting for @DradeAW to confirm if this solves issue #1525 but I think it should. 
I added tests to cover the error which was generated by our encoder not going into extractors instances that are nested in other extractors and removing the numpy scalars from them. 

I added the tests, I saw that they were failing and the modified the code so they pass. Once @DradeAW confirms that this works I will remove the log and this will be ready to merge.

Edit II: OK, this is ready now. 


Patch:
diff --git a/src/spikeinterface/core/base.py b/src/spikeinterface/core/base.py
index dcbed5b87..42872a24a 100644
--- a/src/spikeinterface/core/base.py
+++ b/src/spikeinterface/core/base.py
@@ -1,4 +1,5 @@
 from pathlib import Path
+from typing import Union, Optional
 import importlib
 import warnings
 import weakref
@@ -281,7 +282,7 @@ class BaseExtractor:
             other._preferred_mp_context = self._preferred_mp_context
 
     def to_dict(self, include_annotations=False, include_properties=False,
-                relative_to=None, folder_metadata=None):
+                relative_to=None, folder_metadata=None) -> dict:
         """
         Make a nested serialized dictionary out of the extractor. The dictionary produced can be used to re-initialize 
         an extractor using load_extractor_from_dict(dump_dict)
@@ -348,7 +349,7 @@ class BaseExtractor:
         return dump_dict
 
     @staticmethod
-    def from_dict(d, base_folder=None):
+    def from_dict(dictionary: dict, base_folder: Optional[Union[Path, str]] = None) -> "BaseExtractor":
         """
         Instantiate extractor from dictionary
 
@@ -364,14 +365,14 @@ class BaseExtractor:
         extractor: RecordingExtractor or SortingExtractor
             The loaded extractor object
         """
-        if d['relative_paths']:
+        if dictionary['relative_paths']:
             assert base_folder is not None, 'When  relative_paths=True, need to provide base_folder'
-            d = _make_paths_absolute(d, base_folder)
-        extractor = _load_extractor_from_dict(d)
-        folder_metadata = d.get('folder_metadata', None)
+            dictionary = _make_paths_absolute(dictionary, base_folder)
+        extractor = _load_extractor_from_dict(dictionary)
+        folder_metadata = dictionary.get('folder_metadata', None)
         if folder_metadata is not None:
             folder_metadata = Path(folder_metadata)
-            if d['relative_paths']:
+            if dictionary['relative_paths']:
                 folder_metadata = base_folder / folder_metadata
             extractor.load_metadata_from_folder(folder_metadata)
         return extractor
@@ -480,11 +481,12 @@ class BaseExtractor:
                                  relative_to=relative_to,
                                  folder_metadata=folder_metadata)
         file_path = self._get_file_path(file_path, ['.json'])
-        
+    
         file_path.write_text(
             json.dumps(dump_dict, indent=4, cls=SIJsonEncoder),
             encoding='utf8',
         )
+    
 
     def dump_to_pickle(self, file_path=None, include_properties=True,
                        relative_to=None, folder_metadata=None):
@@ -809,7 +811,7 @@ class BaseExtractor:
         return cached
 
 
-def _make_paths_relative(d, relative):
+def _make_paths_relative(d, relative) -> dict:
     relative = str(Path(relative).absolute())
     func = lambda p: os.path.relpath(str(p), start=relative)
     return recursive_path_modifier(d,  func, target='path', copy=True)
diff --git a/src/spikeinterface/core/core_tools.py b/src/spikeinterface/core/core_tools.py
index 7a4b26403..af5147705 100644
--- a/src/spikeinterface/core/core_tools.py
+++ b/src/spikeinterface/core/core_tools.py
@@ -1,7 +1,7 @@
 from pathlib import Path
+from typing import Union
 import os
 import sys
-import warnings
 import datetime
 import json
 from copy import deepcopy
@@ -110,25 +110,30 @@ class SIJsonEncoder(json.JSONEncoder):
 
     # This machinery is necessary for overriding the default behavior of the json encoder with keys
     # This is a deep issue that goes deep down to cpython: https://github.com/python/cpython/issues/63020
+    # This object is called before encoding (so it pre-processes the object to not have numpy scalars)
     def iterencode(self, obj, _one_shot=False):
-        return super().iterencode(self.remove_numpy_scalar_from_object(obj), _one_shot=_one_shot)
+        return super().iterencode(self.remove_numpy_scalars(obj), _one_shot=_one_shot)
+
+    def remove_numpy_scalars(self, object):
+        from spikeinterface.core.base import BaseExtractor
 
-    def remove_numpy_scalar_from_object(self, object):
         if isinstance(object, dict):
-            return self.remove_python_scalar_in_dict(object)
-        elif isinstance(object, list):
-            return self.remove_numpy_scalar_in_list(object)
+            return self.remove_numpy_scalars_in_dict(object)
+        elif isinstance(object, (list, tuple, set)):
+            return self.remove_numpy_scalars_in_list(object)
+        elif isinstance(object, BaseExtractor):
+            return self.remove_numpy_scalars_in_dict(object.to_dict())
         else:
             return object.item() if isinstance(object, np.generic) else object
 
-    def remove_numpy_scalar_in_list(self, list_: list) -> list:
-        return [self.remove_numpy_scalar_from_object(obj) for obj in list_]
+    def remove_numpy_scalars_in_list(self, list_: Union[list, tuple, set]) -> list:
+        return [self.remove_numpy_scalars(obj) for obj in list_]
 
-    def remove_python_scalar_in_dict(self, dictionary: dict) -> dict:
+    def remove_numpy_scalars_in_dict(self, dictionary: dict) -> dict:
         dict_copy = dict()
         for key, value in dictionary.items():
-            key = self.remove_numpy_scalar_from_object(key)
-            value = self.remove_numpy_scalar_from_object(value)
+            key = self.remove_numpy_scalars(key)
+            value = self.remove_numpy_scalars(value)
             dict_copy[key] = value
 
         return dict_copy
@@ -730,7 +735,7 @@ def is_dict_extractor(d):
     return is_extractor
 
 
-def recursive_path_modifier(d, func, target='path', copy=True):
+def recursive_path_modifier(d, func, target='path', copy=True) -> dict:
     """
     Generic function for recursive modification of paths in an extractor dict.
     A recording can be nested and this function explores the dictionary recursively


Test patch:
diff --git a/src/spikeinterface/core/tests/test_jsonification.py b/src/spikeinterface/core/tests/test_jsonification.py
index 7ddd2d7c6..349e8d460 100644
--- a/src/spikeinterface/core/tests/test_jsonification.py
+++ b/src/spikeinterface/core/tests/test_jsonification.py
@@ -3,6 +3,7 @@ import json
 import pytest
 import numpy as np
 
+from spikeinterface.core.base import BaseExtractor
 from spikeinterface.core.core_tools import SIJsonEncoder
 from spikeinterface.core.generate import generate_recording, generate_sorting
 
@@ -33,6 +34,19 @@ def numpy_array_float():
 def numpy_array_bool(numpy_array_float):
     return numpy_array_float > 0.5
 
+@pytest.fixture(scope="module")
+def dictionary_with_numpy_scalar_keys(numpy_array_integer, numpy_array_float, numpy_array_bool):
+    numpy_integer_scalar = numpy_array_integer[0]
+    numpy_float_scalar = numpy_array_float[0]
+    numpy_boolean_scalar = numpy_array_bool[1]
+    
+    dictionary = {
+        numpy_integer_scalar: "value_of_numpy_integer_scalar",
+        numpy_float_scalar: "value_of_numpy_float_scalar",
+        numpy_boolean_scalar: "value_of_boolean_scalar",
+    }
+
+    return dictionary
 
 def test_numpy_array_encoding(numpy_array_integer, numpy_array_float, numpy_array_bool):
     json.dumps(numpy_array_integer, cls=SIJsonEncoder)
@@ -66,17 +80,8 @@ def test_encoding_numpy_scalars_values(numpy_array_integer, numpy_array_float, n
     json.dumps(dict_with_numpy_scalar_values, cls=SIJsonEncoder)
 
 
-def test_encoding_numpy_scalars_keys(numpy_array_integer, numpy_array_float, numpy_array_bool):
-    numpy_integer_scalar = numpy_array_integer[0]
-    numpy_float_scalar = numpy_array_float[0]
-    numpy_boolean_scalar = numpy_array_bool[1]
-
-    dict_with_numpy_scalar_keys = {
-        numpy_integer_scalar: "first_string",
-        numpy_float_scalar: "second_string",
-        numpy_boolean_scalar: "third_string",
-    }
-    json.dumps(dict_with_numpy_scalar_keys, cls=SIJsonEncoder)
+def test_encoding_numpy_scalars_keys(dictionary_with_numpy_scalar_keys):
+    json.dumps(dictionary_with_numpy_scalar_keys, cls=SIJsonEncoder)
 
 
 def test_encoding_numpy_scalars_keys_nested(numpy_array_integer, numpy_array_float, numpy_array_bool):
@@ -88,7 +93,7 @@ def test_encoding_numpy_scalars_keys_nested(numpy_array_integer, numpy_array_flo
     json.dumps(dict_with_nested_numpy_scalars, cls=SIJsonEncoder)
 
 
-def test_encoding_numpy_scalars_keys_nestes_mixed(numpy_array_integer, numpy_array_float, numpy_array_bool):
+def test_encoding_numpy_scalars_keys_nested_mixed(numpy_array_integer, numpy_array_float, numpy_array_bool):
     numpy_integer_scalar = numpy_array_integer[0]
     numpy_float_scalar = numpy_array_float[0]
     numpy_boolean_scalar = numpy_array_bool[1]
@@ -112,7 +117,7 @@ def test_numpy_dtype_encoding():
 
 
 def test_numpy_dtype_alises_encoding():
-    # People tend to use as a dtype instead of the proper classes
+    # People tend to use this a dtype instead of the proper classes
     json.dumps(np.int32, cls=SIJsonEncoder)
     json.dumps(np.float32, cls=SIJsonEncoder)
     json.dumps(np.bool_, cls=SIJsonEncoder)  # Note that np.bool was deperecated in numpy 1.20.0
@@ -126,3 +131,62 @@ def test_recording_encoding(numpy_generated_recording):
 def test_sorting_encoding(numpy_generated_sorting):
     sorting = numpy_generated_sorting
     json.dumps(sorting, cls=SIJsonEncoder)
+
+
+class DummyExtractor(BaseExtractor):
+    def __init__(self, attribute, other_extractor=None, extractor_list=None, extractor_dict=None):
+        self.an_attribute = attribute
+        self.other_extractor = other_extractor
+        self.extractor_list = extractor_list
+        self.extractor_dict = extractor_dict
+        self.is_dumpable = True
+
+        self._kwargs = {
+            "attribute": attribute,
+            "other_extractor": other_extractor,
+            "extractor_list": extractor_list,
+            "extractor_dict": extractor_dict,
+        }
+
+
+@pytest.fixture(scope="module")
+def nested_extractor(dictionary_with_numpy_scalar_keys):
+    inner_extractor = DummyExtractor(attribute=dictionary_with_numpy_scalar_keys)
+    extractor = DummyExtractor(attribute="a random attribute", other_extractor=inner_extractor)
+
+    return extractor
+
+
+@pytest.fixture(scope="module")
+def nested_extractor_list(dictionary_with_numpy_scalar_keys):
+    inner_extractor1 = DummyExtractor(attribute=dictionary_with_numpy_scalar_keys)
+    inner_extractor2 = DummyExtractor(attribute=dictionary_with_numpy_scalar_keys)
+
+    extractor = DummyExtractor(attribute="a random attribute", extractor_list=[inner_extractor1, inner_extractor2])
+
+    return extractor
+
+
+@pytest.fixture(scope="module")
+def nested_extractor_dict(dictionary_with_numpy_scalar_keys):
+    inner_extractor1 = DummyExtractor(attribute=dictionary_with_numpy_scalar_keys)
+    inner_extractor2 = DummyExtractor(attribute=dictionary_with_numpy_scalar_keys)
+
+    extractor = DummyExtractor(
+        attribute="a random attribute",
+        extractor_dict=dict(inner_extractor1=inner_extractor1, inner_extractor2=inner_extractor2),
+    )
+
+    return extractor
+
+
+def test_encoding_numpy_scalars_within_nested_extractors(nested_extractor):
+    json.dumps(nested_extractor, cls=SIJsonEncoder)
+
+
+def test_encoding_numpy_scalars_within_nested_extractors_list(nested_extractor_list):
+    json.dumps(nested_extractor_list, cls=SIJsonEncoder)
+
+
+def test_encoding_numpy_scalars_within_nested_extractors_dict(nested_extractor_dict):
+    json.dumps(nested_extractor_dict, cls=SIJsonEncoder)