navis icon indicating copy to clipboard operation
navis copied to clipboard

read_swc: strange behaviour when file path contains typo

Open schlegelp opened this issue 2 years ago • 2 comments

navis.read_swc will throw very unhelpful ValueErrors (via pandas) when the input is a non-existing (file)path (e.g. in case of a typo). Try for example this:

_ = navis.read_swc('this/file/does/not/exist.swc')

The reason being that strings are first tested for being folder and files, and failing that will be treated as SWC strings without any additional checks and balances.

I had a look at both BaseReader and SwcReader, and the easiest way, I think, to catch these would be add a quick check right at the start of navis.read_swc (before SwcReader.read_any is called). Something like this:

if isinstance(f, str) and '\n' not in f and not utils.is_url(f):
    # If this looks like a path, check if it is a valid one
    p = Path(f).expanduser()
    if not p.is_dir() and not p.is_file():
        raise ValueError(f'"{f}" looks like a directory or filepath but '
                         'does not appear to exist.')

Now this seems to work reasonable well but I'm worried that there is some corner case I'm not thinking about. Thoughts @clbarnes ?

schlegelp avatar Apr 14 '22 17:04 schlegelp

It's not impossible to have paths with newlines or which look like URLs, but they're pretty rare.

As an aside, as there are lots of failure modes for this very general function, maybe it's worth some bespoke errors replacing some of the ValueErrors?

clbarnes avatar Apr 14 '22 18:04 clbarnes

I fully agree. That said: the BaseReader class is already a pretty complex thing and I'm a bit reluctant to add to that.

My suggested solution would still correctly check most file paths which is an improvement over what's there right now. I'm tempted to add this and see if I can add some more useful error messages (e.g. for malformed SWC files).

schlegelp avatar Apr 15 '22 07:04 schlegelp