MorphIO
MorphIO copied to clipboard
BlueBrain
Add more options for ASC and SWC loaders
I did not add options for all checks, just for the ones that might be needed for TMD.
Also, I did not change the HDF5 reader at all for two reasons:
- its implementation is deeply designed according to the specifications given in https://github.com/BlueBrain/morphology-documentation/blob/main/source/h5v1.rst, so it would be hard and dangerous to remove some checks.
- the morphologies written in this format mainly come from MorphIO which do not allow to write morphologies that do not follow its specifications, so I don't think we will ever find a HDF5 file containing a morphology that can not be loaded with the current loader.
@matz-e @eleftherioszisis @mgeplf Could you tell me if this PR looks relevant to you (we can speak about implementation details later)? If it does I will add some docs about these options. Thanks
Could you tell me if this PR looks relevant to you (we can speak about implementation details later)? If it does I will add some docs about these options. Thanks
I am in favor of adding these options if they aid in processing morphologies with artifacts.
However, my main concern is about the repercussions of using these options and then converting between formats. I believe we need way more tests to ensure that the converters will not break when using these options.
However, my main concern is about the repercussions of using these options and then converting between formats. I believe we need way more tests to ensure that the converters will not break when using these options.
I think they will because the HDF5 format is very strict and I did not change much in this one because I think the HDF5 files are mainly created using MorphIO, even though they could be created in another way (and also because the HDF5 part of MorphIO is harder to change :innocent: ). But I don't think it's really an issue if these flawed morphologies can not be written before they are fixed. But I am going to add some tests to see which cases are possible and which are not.
I am in favor of adding these options if they aid in processing morphologies with artifacts.
However, my main concern is about the repercussions of using these options and then converting between formats. I believe we need way more tests to ensure that the converters will not break when using these options.
Same. Also, this should not create any illusion that all our software can run with non-fixed morphologies. We have a pretty rigid idea of how a morphology should look like (exactly 1 soma, e.g.)
Same. Also, this should not create any illusion that all our software can run with non-fixed morphologies. We have a pretty rigid idea of how a morphology should look like (exactly 1 soma, e.g.)
That's why I added warnings in all irregular cases, but you think it's not enough?
To be more clear I improved the conversion test to check that morphologies that can be written can then be read without any specific option. This means that the irregular morphologies can be loaded with the new options BUT when they can be written (mainly for soma issues) they are automatically fixed (not in a smart way but just to make it work). This only happens for weird somata (e.g. multiple somata or somata with bifuractions). So the user gets a warning but if he is able to write it, whether he manually fixes the morphology or he lets MorphIO to fix it automatically if it is possible, then MorphIO will be able to load it with the default loader afterwards.
but you think it's not enough?
I'm more concerned. Let's hope it's enough. People being aware helps a lot already.
but you think it's not enough?
I'm more concerned. Let's hope it's enough. People being aware helps a lot already.
Ok :)
@mgeplf @eleftherioszisis @matz-e up :) I rebased and it covers everything I need for now so for me it's ready.
Thanks, I'm still far behind on fixing things :( I will add it to the list of things to do....
Thanks, I'm still far behind on fixing things :( I will add it to the list of things to do....
Ahah no problem, at least now it will be in your todo list :stuck_out_tongue_winking_eye: Thanks!