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

Set !SIM.file.error_on_missing_file to True by default #458

Open
teutoburg opened this issue Aug 26, 2024 · 1 comment
Open

Set !SIM.file.error_on_missing_file to True by default #458

teutoburg opened this issue Aug 26, 2024 · 1 comment
Assignees
Labels
refactor Implementation improvement

Comments

@teutoburg
Copy link
Contributor

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.

@teutoburg teutoburg added the refactor Implementation improvement label Aug 26, 2024
@teutoburg teutoburg self-assigned this Aug 26, 2024
@teutoburg teutoburg moved this from 🆕 New to 📋 Backlog in ScopeSim-development Aug 26, 2024
@hugobuddel
Copy link
Collaborator

Agreed, having !SIM.file.error_on_missing_file set to False leads to annoying problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Implementation improvement
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants