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

LJMError should be moved into its own exception module #9

Open
jbiams77 opened this issue Oct 31, 2022 · 2 comments
Open

LJMError should be moved into its own exception module #9

jbiams77 opened this issue Oct 31, 2022 · 2 comments

Comments

@jbiams77
Copy link

jbiams77 commented Oct 31, 2022

by defining the same LJMError class in the module where it is used, there exists a chicken/egg problem where the LJMError cannot be used to catch exception when importing LJM library.

For example:

try:
    from labjack import ljm
except labjack.ljm.ljm.LJMError as exception:
    raise LabjackLibraryNotInstalled from exception

cannot work because the LJMError is defined in the same module. A better practice is to seperate the exception class into it's own module that only depends on Python standard library, this ensures Labjack package exceptions can be used for module imports that wrap library code, just as labjack-ljm-python wraps the LJM library that is required to be installed.

Another issue is that the _loadLibrary() function catches the LJMError exception, prints the error, then returns None. Why? If the python package doesn't work without the LJM library, then it shouldn't handle the exception, it should just raise it and let the calling module handle the exception.

Please remove from _loadLibrary():

    except LJMError:
        ljme = sys.exc_info()[1]
        print(str(type(ljme)) + ": " + str(ljme))
        return None
@gavinmccabe
Copy link

I had this same issue. I attempted a quick solution in #19 to fix this but haven't gotten any responses. Either way, #19 has a hacky workaround for any others reading this...

@davelopez01
Copy link
Contributor

Sorry for not replying earlier and thank you for bringing up this issue. We are currently working on T8 release tasks. This is on our list of issues to look into soon.

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

No branches or pull requests

3 participants