Extension delete on recompute

[samuelgarcia]

@chrishalcrow : this should fix the issue we discussed when recomputing something all children dependency are not valid anymore and have to deleted. Here a simple implementation of this idea.

[chrishalcrow]

Nice, works for me!

Maybe we could warn the user in some cases? E.g. deleting random spikes deletes everything! Or tell them what's happening:
"{child}, which depends on {parent}, has also being deleted."
?

[samuelgarcia]

@alejoe91 : what do you think of warning or print when cascade deleteting on re compute ?

[zm711]

For me I mean I wonder what the warning does? Because you're not going to say delete then offer a y|n in the terminal right. I think it is probably more important to have strong docstrings and documentation to explain that extensions now have parent-child relations and so deleting the parent necessitates deleting the child, no? It might be nice instead in my opinion to offer a function like get_all_extensions_relationships that prints out the dependencies of my current extensions so that I can understand how things interact before I as a user delete them.

For me the warning only helps if I can stop it when I realize that deleting random_spikes deletes waveforms too... But in this case this is just a print that we are putting as a warning. But that's just my two cents.

[chrishalcrow]

I know that I would be confused if I deleted "random_spikes" and everything vanished. I might not realise that the other extensions were deleted when I deleted random_spikes. So the warning highlights this fact.

But yeah, if this was documented well that could work too!

Another idea: now that we have these relationships we could have a "compute_parents" option on the compute() method?

[zm711]

I know that I would be confused if I deleted "random_spikes" and everything vanished. I might not realise that the other extensions were deleted when I deleted random_spikes. So the warning highlights this fact.

I think that is fair for newer users. I think it will also really annoy them that the warning is warning them after it is already done, but it is true that it will at least them know why they are now missing more than just what they thought they were deleting. So I can see it being helpful. I guess I could go either way.

Another idea: now that we have these relationships we could have a "compute_parents" option on the compute() method?

I think @alejoe91 wanted that too! We discussed that a bit here #2504, but Sam was against it because he wanted to keep things as explicit as possible.

[zm711]

I'm just wondering if we can get a proper docstring here. For future contributors that haven't been a part of the transition to SortingAnalyzer I think it would be nice. For example the parent-child in neo can be pretty confusing for new people so if we document it now I think future developers will thank us.

[samuelgarcia]

OK I will do this but this function will be private. I think it is internal no ?

[samuelgarcia]

done

[zm711]

I'm just imagining some young developer in 100 years working on spikeinterface and wishing all the private functions were fully documented.

[alejoe91]

Hopefully there will be a 1.0 by then ahahah

[samuelgarcia]

Finally I choose the silently deletion.
Recomputing an extension is a corner case. Unless you are trying parameters.