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

imread Should Raise If Path Doesn't Exist #14095

Open
zhanwenchen opened this issue Mar 18, 2019 · 7 comments
Open

imread Should Raise If Path Doesn't Exist #14095

zhanwenchen opened this issue Mar 18, 2019 · 7 comments

Comments

@zhanwenchen
Copy link

zhanwenchen commented Mar 18, 2019

System information (version)
  • OpenCV => 4.0.1
  • Operating System / Platform => MacOS 10.14 Mojave
  • Compiler => Xcode 10.1
Detailed description

cv2.imread doesn't check file existence and reads nonexistent files into a NoneType object. In comparison, matplotlib.pyplot.imread and PIL both throw.

Steps to reproduce
import cv2

nonetype_thingy = cv2.imread('nonexistent_path.jpg') # this is somehow legal
cv2.imshow('lel', nonetype_thingy) # this will throw a "cv2.error: OpenCV(4.0.1) /Users/username/opencv/modules/highgui/src/window.cpp:352: error: (-215:Assertion failed) size.width>0 && size.height>0 in function 'imshow'"
@alalek
Copy link
Member

alalek commented Mar 19, 2019

Mentioned API assumes that empty (None in Python) result is returned.
Changing this convention will break existed OpenCV applications.

@LaurentBerger
Copy link
Contributor

@alalek What's about to add :
static const size_t CV_IO_IMREAD_EXCEPTION = utils::getConfigurationParameterSizeT("OPENCVIO_IMREAD_EXCEPTION", 0);

and new user can set to 1 if they want exception ?

@LJXLJXLJX
Copy link

LJXLJXLJX commented Apr 30, 2019

Yes, without raising exceptions here, bugs can't be located in time.
It may takes a lot of time to debug manually to detect this tiny mistake.
I think this design does not meet the user-friendly concept.

@zhanwenchen
Copy link
Author

@alalek I much prefer breaking changes to mismatched expectations. Why would this "break existed OpenCV applications?" Also why are those applications using this special fake "NoneType" thing anyway?

@mpolicki
Copy link

Current imread documentation says:

If the image cannot be read (because of missing file, improper permissions, unsupported or invalid format), the function returns an empty matrix

From a software-development-best-practices standpoint, this is completely unacceptable. Nowhere will you find advice like the following:

If an error occurs, don't report it to the caller, just fail silently.

Instead, the advice you typically find is something like (emphasis added):

Check input and fail on nonsensical input or invalid state as early as possible, preferably with an exception or error response that will make the exact problem clear to your caller.

So, at the very least, imread needs to report to the caller which error it has encountered, if any. The mechanism of this error reporting should be appropriate to the language. In C++, that mechanism is exceptions. Therefore, imread should throw the appropriate exception if it encounters an error.

@sturkmen72
Copy link
Contributor

i am working on #17753 and planning to work also on error handling. now waiting a review to continue

@alalek
Copy link
Member

alalek commented Aug 18, 2020

imread() comes from legacy C API which don't have C++ exception or similar error handling.
Adding exceptions into imread() directly break many applications.
So API itself should be evolved.

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

No branches or pull requests

6 participants