labjack-ljm-python icon indicating copy to clipboard operation
labjack-ljm-python copied to clipboard

Remove top-level error handling in _loadLibrary

Open gavinmccabe opened this issue 1 year ago • 1 comments

Removing this try block enables LJM loading errors to be percolated up to the user and allows proper handling of them in a reasonable context. As of now, the program will continue and the import error will be printed to stdout. In some logging contexts these errors won't be seen and won't stop the program, giving a more cryptic error about the library None having no attribute openS. To detect the error right now, we would need to do something like

from contextlib import redirect_stdout

with redirect_stdout(None):
    from labjack import ljm

which is fairly intuitive.Instead, this update allows users to choose how to handle the error within standard Python error handling (and choosing to ignore as the current version does):

try:
    from labjack import ljm
except LJMError as err:
    # handle error in python

I believe the labjack module should give users the most options to handle errors as they see fit and not require hacky solutions such as redirecting stdout to provide end-users proper error messages in a standard location.

gavinmccabe avatar Jan 24 '24 20:01 gavinmccabe

Thank you for your contribution.

We evaluated your commit and we are concerned about it raising LJMError in the _loadLibrary function. Since the LJMError class is in the ljm module with _loadLibrary, the user cannot import and directly catch LJMError if _loadLibrary raises an exception.

For example, this code will not run as wanted:

try:
    from labjack import ljm
except LJMError as err:
    # handle error in python

And raises an exception like:

NameError: name 'LJMError' is not defined. Did you mean: 'OSError'?

The issue you are trying to resolve is on our list of things to look into, but currently we are working on T8 tasks.

davelopez01 avatar Feb 01 '24 20:02 davelopez01