navis-org / navis

Python library for analysis of neuroanatomical data.
https://navis-org.github.io/navis/
GNU General Public License v3.0
83 stars 33 forks source link

read_swc: strange behaviour when file path contains typo #91

Open schlegelp opened 2 years ago

schlegelp commented 2 years ago

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 ?

clbarnes commented 2 years ago

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?

schlegelp commented 2 years ago

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).