Analyzer extension exit status

[jonahpearl]

Fixes #3329

Here's a first pass at adding information about an extension's completion (or not) to its metadata.

This PR adds a run_info.json file to each extension's folder. This contains keys:

• run_completed (whether or not the call to AnalyzerExtension._run() finished),
• data_loadable (whether or not AnalyzerExtension.load_data() can be called without an error)
• runtime_s (the runtime of AnalyzerExtension._run() in seconds).

The core logic is implemented in AnalyzerExtension.run() and wraps the call to the extension-specific _run(). Then when AnalyzerExtension.get_extension() is called (and redirects to AnalyzerExtension.load() internally), if the run wasn't completed, it simply returns None as if the extension doesn't exist, and the user would be able to catch that and re-run the extension.

There is a mild complication with extensions that use the run_node_pipeline if called through compute_several_extensions, because then AnalyzerExtension.run() is never actually called. Instead, it looks like AnalyzerExtension.save() is used, so I added the relevant lines there to catch that, and assume that if the code makes it there, the run is completed.

TODO:

• I'm not sure if the implementations in merge() and copy() are correct, I don't totally understand when those methods are expected to be called / what's wrapping them.
• Write some tests?

Notes:

• It is a bit of a slow-down to check if the data are loadable after each run. I'm ambivalent as to whether that check is actually useful, especially since data corruption is more likely to happen sometimes significantly after the run, not immediately after.
• It seemed more reasonable to add a separate run_info.json file than trying to cram this into info.json, since that is more about the code itself.

[samuelgarcia]

Hi Jonah.
Thanks a lot. this looks like really cool.
I will have a deeper look into this.

data_loadable and run_completed are a bit redundant. (unles the computation is done but the wring failed.)

[jonahpearl]

I also fixed the edge case where (for some reason) the run completes, but then the data file gets deleted; it will now raise a warning upon loading the sorting analyzer, and return None on calls to get_extension(), since the extension is no longer usable and should be re-computed as if it had never been.

[jonahpearl]

Also — I still think my naive / default usage of this code would be to check has_extension(), and if that's true, assume everything is ok. Sure, the "check for None" pattern can be shown in the docs, but it's still odd that has_extension() now essentially means, is there a folder, regardless of if there's data. It looks like internally, sometimes it's used to mean, "is there a folder", like here, but then sometimes it means, "is there usable data", like here or here or here. One could imagine having a private function for "is there a folder here", and either getting rid of the public-facing version in favor of get_extension and checking none, or just making has_extension essentially do the none check for you.

Just taking the third example:

if sorting_analyzer.has_extension("spike_amplitudes"):
    peak_amplitudes = sorting_analyzer.get_extension("spike_amplitudes").get_data()
else:
    peak_amplitudes = None

right now, that will fail if the extension doesn't have data, since has_extension() will come back True but then get_extension() will return None and the call to get_data() will raise an AttributeError. I guess currently the preferred implementation would be

spike_amps = sorting_analyzer.get_extension("spike_amplitudes")
if spike_amps:
    peak_amplitudes = spike_amps.get_data()
else:
    peak_amplitudes = None

(or with a tertiary operator plus assignment expression it can fit in one line)

peak_amplitudes = _ext.get_data() if (_ext := sorting_analyzer.get_extension("spike_amplitudes")) else None

but the pattern still just feels confusing 🤷

[alejoe91]

@samuelgarcia this is good to merge on my side. I added a back-compatibility logic for loading folders/zarr produced prior to this change

[samuelgarcia]

OK for me but if we want the total run ttime then suming of run_time will be sring because this run time is shared.

[alejoe91]

This is the best estimate we can get :)

[samuelgarcia]

Merci beaucoup Jonah et Alessio.