ITK icon indicating copy to clipboard operation
ITK copied to clipboard

Add checks for unsupported characters within image path and output path

Open Pfleiderer-Adrian opened this issue 1 year ago • 5 comments

Description

I encountered an issue while attempting to load images using the ITK / SimpleITK framework in my Python script. The traceback indicates that the problem arises when attempting to create an IO object for reading an image file that contains Umlauts (ä, ü, ö) in its path. See: #4388

Providing a more descriptive error message indicating that the path cannot be read due to special characters, such as Umlauts, would be helpful for users to understand and address the issue.

Changes

I have added a simple filepath check for illegal characters in the _NewImageReader and imwrite routines. Additionally, for the imwrite routine, a check was added to verify if the output directory exists.

PR Checklist

Pfleiderer-Adrian avatar Jan 10 '24 12:01 Pfleiderer-Adrian

There may be something in the SWIG layer that can be down with the conversion. There is some information here: https://www.swig.org/Doc3.0/Python.html#Python_nn77

That needs to be gone through.

blowekamp avatar Jan 10 '24 17:01 blowekamp

@Pfleiderer-Adrian thanks for having a look at this. A few comments:

  • Commit 7304267 does not address a bug, but a STYLE change.
  • Please, add a meaningful commit message body in line with ITK commit message practices.

jhlegarreta avatar Jan 10 '24 20:01 jhlegarreta

The second commit should be squashed into the first one.

dzenanz avatar Jan 10 '24 21:01 dzenanz

Thank you for your feedback! I'll post an update this weekend. :)

Pfleiderer-Adrian avatar Jan 12 '24 09:01 Pfleiderer-Adrian

Thank you for diving into this issue, @Pfleiderer-Adrian Do I understand correctly that this issue (#4388) is only a problem for Windows users, not for Linux users? (Specifically, only those Windows users that do not have UTF-8 support enabled and selected a locale that does not support the filename?) I'm asking, because it feels too strict to only accept ASCII characters from now, for all users.

So is it really the intention to reject any non-ASCII character in file names, for any user? Aren't we then throwing the baby out with the bath water?

Would it be an idea to only do those checks after trying to use imageIO, and only when imageIO has really failed to do its job?

N-Dekker avatar Jan 12 '24 10:01 N-Dekker