Make sanitize section deletion non-recursive

[eleftherioszisis]

When a zero length section is encountered, the sanitization deleted the entire subtree.

This change limits the deletion to the zero-length section only. If a root section with zero length is encountered, an error will be raised.

#99 #105 #106

[eleftherioszisis]

Before fixing the rest of the tests that use morphologies with zero length root sections, I wonder if raising an error is the desirable way to go.

Alternatively, we could skip the root sections that have zero lengths and emit a warning. This less aggressive approach would allow to sanitize a morphology even if it has zero length root sections. The root sections would be left untouched.

@mgeplf @FrancescoCasalegno @lidakanari @arnaudon @adrien-berchet

[adrien-berchet]

But that we would end up with sanitized neurites (the ones whose root section has length > 0) and with not sanitized neurites (the ones with 0-length root section), which is a bit weird, isn't it? Or maybe we could still sanitize all the sections except the root when it has length = 0?

[eleftherioszisis]

But that we would end up with sanitized neurites (the ones whose root section has length > 0) and with not sanitized neurites (the ones with 0-length root section), which is a bit weird, isn't it? Or maybe we could still sanitize the all sections except the root when it has length = 0?

I rephrased the last sentence. I meant to sanitize the length of all sections but the root ones. Sorry, for the confusion.

[adrien-berchet]

But that we would end up with sanitized neurites (the ones whose root section has length > 0) and with not sanitized neurites (the ones with 0-length root section), which is a bit weird, isn't it? Or maybe we could still sanitize all the sections except the root when it has length = 0?

I rephrased the last sentence. I meant to sanitize the length of all sections but the root ones. Sorry, for the confusion.

Ah ok ok, sorry I miss understood. I agree with that then :)

[FrancescoCasalegno]

Alternatively, we could skip the root sections that have zero lengths and emit a warning. This less aggressive approach would allow to sanitize a morphology even if it has zero length root sections. The root sections would be left untouched.

Hi @eleftherioszisis ! As far as I understand there is no 100% clear reason why cannot just have 0-length root sections (NEURON or something else may break?), but as we discussed yesterday it seems dangerous to leave them there.

Now, how do we handle those 0-length root sections during sanitize? These seems to be the options:

1. We leave the 0-length root sections there, and potentially just log a Warning so the user. --- But then NEURON or other BBP tool may fail.
2. We drop the entire sub-tree. --- But this makes the output morphology almost unusable for all practical purposes.
3. We extend the 0-length to be of min_len > 0. --- But this may not be what all users want, so better to do it elsewhere (see Fix 0-length root sections in CheckNeurites task morphology-workflows#31).

So, since there was no agreement on the what is the most "logical" solution to the issue, I think that raising an Error may be a good solution. An alternative would be to create a parameter if_zero_root_len that defines how to behave (e.g. if_zero_root_len: Literal["raise", "ignore", "extend"]) but @mgeplf said we don't want to have thousands of parameters (also: what would be the default of this parameter anyway? we are back to starting point...)

[eleftherioszisis]

So, since there was no agreement on the what is the most "logical" solution to the issue, I think that raising an Error may be a good solution. An alternative would be to create a parameter if_zero_root_len that defines how to behave (e.g. if_zero_root_len: Literal["raise", "ignore", "extend"]) but @mgeplf said we don't want to have thousands of parameters (also: what would be the default of this parameter anyway? we are back to starting point...)

Sure, I prefer raising an error to adding a parameter too. I will keep it strict, thanks.

[mgeplf]

I think that raising an Error may be a good solution.

I second raising an error; warning tend to get ignored, IMO, and parameters opens the pandoras box for having options for all the sanitization.

[eleftherioszisis]

The changes in this module are just some refactoring mainly for not filtering the exception when an error is thrown and some formatting. You can ignore them.