The utils.find_file() function currently by default returns None if a file isn't found, only raises if !SIM.file.error_on_missing_file is set to True (as it's already done during testing). Looking at the few places where this function is actually used, some of them explicitly check for None being returned, which could also be done more explicitly with a try-except clause. The other case is more problematic: the resulting path from find_file() is immediately opened (either directly or through imported I/O functions), which can lead to generic errors like AttributeError: 'NoneType' object has no attribute 'read', where it's not immediately obvious what the problem is (yes, find_file() by default logs a message in this case, but this gets easily lost in the verbose stack trace). In those cases, raising an explicit error about the file not being found is IMHO preferable to waiting for the I/O operation to fail (which will happen anyway).
So, I propose turning the existing if path is None etc. into try-excepts and setting !SIM.file.error_on_missing_file by default to True. Then we'll see if there are any unexpected consequences, which we probably should know about anyway. I also propose utils.find_file() should then raise FileNotFoundError instead of ValueError.
The
utils.find_file()
function currently by default returnsNone
if a file isn't found, only raises if!SIM.file.error_on_missing_file
is set toTrue
(as it's already done during testing). Looking at the few places where this function is actually used, some of them explicitly check forNone
being returned, which could also be done more explicitly with a try-except clause. The other case is more problematic: the resulting path fromfind_file()
is immediately opened (either directly or through imported I/O functions), which can lead to generic errors likeAttributeError: 'NoneType' object has no attribute 'read'
, where it's not immediately obvious what the problem is (yes,find_file()
by default logs a message in this case, but this gets easily lost in the verbose stack trace). In those cases, raising an explicit error about the file not being found is IMHO preferable to waiting for the I/O operation to fail (which will happen anyway).So, I propose turning the existing
if path is None
etc. into try-excepts and setting!SIM.file.error_on_missing_file
by default toTrue
. Then we'll see if there are any unexpected consequences, which we probably should know about anyway. I also proposeutils.find_file()
should then raiseFileNotFoundError
instead ofValueError
.