-
Notifications
You must be signed in to change notification settings - Fork 19.5k
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
Minor change: Added tif and tiff to white_list_formats #9187
Conversation
This could be an issue for a lot of people. PIL doesn't work if the tiff has more than 3 channels. |
@Dref360 your call. |
I don't have a counter argument. If you think that it is going to cause more problems than it will solve I understand. Let me know 😄. |
@CharlesAuthier is working in my lab and has more experience on this type of file. |
The only workaround I see is to use Pillow and libtiff instead of PIL. This is a major change so we shouldn't go this way. According to Charles, if you have RGB-IR data (or any multiband data), the output will be deformed by PIL. It won't affect most people. So I would like to merge it, but maybe add a warning in UserWarning('Using \'.tiff\' files with multiple bands will cause distortion.',
'Please verify your output.') |
Something like that 🤔 ? |
@@ -963,6 +963,9 @@ def _recursive_list(subpath): | |||
for fname in files: | |||
is_valid = False | |||
for extension in white_list_formats: | |||
if fname.lower().endswith('.tiff'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will warns a tremendous amount times, isn't it?
I would just do a check before returning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I am pretty sure it won't (I also ran it).
See the docs at http://docs.python.org/2/library/warnings.html:
"Repetitions of a particular warning for the same source location are typically suppressed."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good then!
LGTM for me. I don't think we need to test that the warning is raised? (It's sometime a pain with pytest) |
If you ask me, I reckon it's ok :) |
keras/preprocessing/image.py
Outdated
@@ -963,6 +963,9 @@ def _recursive_list(subpath): | |||
for fname in files: | |||
is_valid = False | |||
for extension in white_list_formats: | |||
if fname.lower().endswith('.tiff'): | |||
warnings.warn('Using \'.tiff\' files with multiple bands will cause distortion.' | |||
'Please verify your output.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this warning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO yes or we shouldn't accept tiff at all.
We'll get complaints that some .tiff are not handled correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok then. The message is missing a space for the period, please fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that. Fixed.
you can temporarily solve this problem by adding 'tif', 'tiff' in the format list in image.py source code |
Hello there,
It just took me some time to understand why "flow_from_directory" didn't work for me, only to realize that the "tif" format was not in the white_list_formats. So, I added it :) .