Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

read_swc: strange behaviour when file path contains typo #91

Open
schlegelp opened this issue Apr 14, 2022 · 2 comments
Open

read_swc: strange behaviour when file path contains typo #91

schlegelp opened this issue Apr 14, 2022 · 2 comments

Comments

@schlegelp
Copy link
Collaborator

schlegelp commented Apr 14, 2022

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants