spikeinterface
spikeinterface copied to clipboard
`.save_to_folder()` fails for the `InjectDriftingTemplatesRecording`
When trying to save a hybrid recording, I get the error TypeError: Object of type DriftingTemplates is not JSON serializable. This makes sense, because DriftingTemplates inherits from Templates which is a dataclass and not a BaseExtractor, so it will not be .to_dict()ed by https://github.com/SpikeInterface/spikeinterface/blob/main/src/spikeinterface/core/base.py#L423 even though it has a .to_dict() method.
I'm not sure how to resolve this? One could maybe make Templates subclass BaseExtractor while still being a dataclass? Although it does feel a bit strange to json-serialize a large array of templates. Anyway, wanted to see what you all think, thanks!
Thanks! One simple option is to make the InjectDriftingTemplatesRecording not JSON serializable, so it will be saved to pkl at all instances
Oh! That sounds good. Happy to make a PR for that -- is it done by implementing check_if_json_serializable(self)? Or would Templates need to inherit from BaseExtractor?
@cwindolf you can just add this at the end of the init:
self._serializability["json"] = False
Mmm the Templates should be json serializable, it has a dict:
https://github.com/SpikeInterface/spikeinterface/blob/ed9d1b5c7de0e3b0af351cbe1454ce74a4fec54c/src/spikeinterface/core/template.py#L250-L261
Could you share a minimal example where this fails?
@h-mayorquin it does have a to_dict() method, but it doesn't inherit from BaseExtractor, so it won't go through the usual code path (see the line I link to in the top post). Sorry, my example is a bit of a pain to share, but I think any example should work. Note that one needs to make sure the sorting used is a dumpable one, else provenance.json just has an error message and you won't get to the point of trying to serialize the templates.
I think for the same reason @alejoe91 just setting self._serializability["json"] is not enough, because Templates is not a BaseExtractor so it won't know how to reconstruct itself -- the code path that's hit when I then later try to load up the full hybrid recording object via sc.load_extractor(my_hybrid_recording_dir / "provenance.json") then goes throughBaseExtractor.load() stuff. I think Templates needs to subclass BaseExtractor, but I don't know enough about how these things work yet...
@h-mayorquin maybe we just need to extend the SIJsonEncoder to support Templates?
@cwindolf no we don't need to inherit from BaseExtractor, because that's for a Recording or Sorting object.
@h-mayorquin let's discuss about this tomorrow!
OK. Let's discuss it tomorrow.
@cwindolf can you test this one #3130? With this PR the provenance is saved to pkl if the recording is not JSON serializable (and this is the case for then InjectDriftingTemplatesRecording)
Thanks @alejoe91 ! This is great. rec.check_serializability(type='json') fails now and there is a provenance.pkl saved. I can access the .drifting_templates.templates_array_moved property and everything. I did this on a .frame_slice() for testing like so:
rec.frame_slice(0, 1000).save_to_folder(scratch_dir)
rec1 = si.load_extractor(scratch_dir / "provenance.pkl")
rec1._kwargs["parent_recording"].drifting_templates.templates_array_moved.shape
@cwindolf you can also do this! (better API)
rec1.get_parent().drifting_templates.templates_array_moved.shape
Thanks, good to know!