Add quiet flag for warnings #261
Conversation
| group = group[root] | ||
| return group[select] | ||
| with warnings.catch_warnings(): | ||
| warnings.simplefilter("ignore" if quiet else "default") |
There was a problem hiding this comment.
I think this resets user-defined filters. You need to not call simplefilter if not quiet. But I may be wrong. Did you check?
There was a problem hiding this comment.
There are some issues with this approach:
- It affects all warnings, even those that may not be related to file structure but, e.g., arise from dependencies.
- It works by changing a global variable. So if this code is used in a multi-threaded context, e.g., in Sciline with Dask, the filter also affects warnings that don't arise from ScippNexus.
Those issues can be somewhat mitigated by filtering on the module where the warning is raised. But you can still get race conditions with multi-threading.
If you want to go with this approach, please add a clear docstring explaining the caveats.
There was a problem hiding this comment.
Would it make sense to consider using logging instead of warnings?
There was a problem hiding this comment.
Not if you want to treat warnings as errors in some contexts.
There was a problem hiding this comment.
There are some issues with this approach
Yes, you make some good points. Do you have a suggestion for an alternative? If not, I guess filtering on the module would already be a big improvement.
There was a problem hiding this comment.
Have we considered putting the filtering code into userland, i.e., not change ScippNexus?
There was a problem hiding this comment.
It's an option, I just thought having a flag would be the most user friendly mechanism.
Using
snx.load(filename, quiet=True)suppressed all warnings raised when loading files.Fixes #260