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

Improve error handling on importing modules for GAMMA #325

Open
caitlinadams opened this issue Dec 3, 2024 · 3 comments
Open

Improve error handling on importing modules for GAMMA #325

caitlinadams opened this issue Dec 3, 2024 · 3 comments

Comments

@caitlinadams
Copy link

I'm still working through getting pyroSAR working with our GAMMA installation. One thing I've noticed is that despite having DISP in the GAMMA directory, pyroSAR hasn't created a module for it in my ~/.pyrosar/gammaparse folder (I have yet to investigate this further). As such, I'm getting non-intuitive errors when trying to use the GAMMA functions.

Specifically, I believe this is because the error handling on the import statement in the GAMMA utils python file is failing silently:

try:
    from .api import diff, disp, isp, lat
except ImportError:
    pass

I confirmed this by running from pyroSAR.gamma.api import diff, disp, isp, lat, which produces

ImportError: cannot import name 'disp' from 'pyroSAR.gamma.api' 

confirming that the disp module is the issue. However, this means that isp never gets imported, and future calls to isp fail, even if disp is not required (which is the case for convert2gamma). I confirmed this by running from pyroSAR.gamma.api import diff, isp, lat which works correctly, allowing me to run dir(isp) and see the module contents.

I believe this is related to #85 -- my enhancement suggestion would be to catch each module import error and let the user know which module failed to import. That way, they can at least be pointed to the problematic module, rather than one that is working (but appears later in the import list).

  • which operating system are you using?
    Rocky Linux release 8.10 (Green Obsidian)

  • which environment is pyroSAR running in?
    Micromamba environment

  • which version of pyroSAR are you using?
    conda forge (pyrosar=0.23.0 in environment.yml file)

  • which function of pyroSAR did you call with which parameters?
    convert2gamma

@johntruckenbrodt
Copy link
Owner

Thanks a lot @caitlinadams. I am sure the GAMMA module needs a refresh. I haven't worked with it for about two years and pyroSAR does not support the latest versions. We have recently obtained a new version and so I will also update pyroSAR to be able to use it. Another related issue is #313 opened by a colleague of mine.

So, you are very welcome to provide the fix you suggest above if you find the time. I agree with your suggestion. However, I could imagine that you run into more errors after this.
I will look into it next week. Once I've opened a PR I will ask you for feedback? Sounds good?
What version (release date) of GAMMA are you using and which modules do you have available?

@caitlinadams
Copy link
Author

No problem, @johntruckenbrodt -- I'm happy to collaborate on the refresh as I can while getting pyroSAR working with our GAMMA version. In the end, I figured out that GAMMA couldn't find some key shared objects for FFTW3 and GDAL, and we solved the import errors by creating symlinks to those objects on our supercomputer. Specifically, the issue was that we couldn't actually run the gamma binaries:

[~]$ GAMMA_SOFTWARE-20230712/DISP/bin/data2geotiff
GAMMA_SOFTWARE-20230712/DISP/bin/data2geotiff: error while loading shared libraries: libgdal.so.20: cannot open shared object file: No such file or directory

If this error had been surfaced by pyroSAR, it would have really helped us with debugging, so perhaps its a matter of displaying any unresolved errors to the user? I found the following code helpful with my debugging, which basically can be used to test if the binary can be run (the output should contain a usage statement if it worked):

from spatialist.ancillary import finder
import re
import subprocess as sp

for module in finder(GAMMA_HOME_PATH, ['[A-Z]*'], foldermode=2):
    print(module)
    for submodule in ['bin', 'scripts']:
        print(f"{module}/{submodule}")
        for cmd in sorted(finder(module+"/"+submodule, [r'^\w+$'], regex=True), key=lambda s: s.lower()):
            command_base = os.path.basename(cmd)
            proc = sp.Popen(command, stdin=sp.PIPE, stdout=sp.PIPE, stderr=sp.PIPE, universal_newlines=True)
            out, err = proc.communicate()
            out += err
            usage = re.search('usage:.*(?=\n)', out)
            if usage is None:
                print("    " + err)
        print("\n\n")

We're using GAMMA_SOFTWARE-20230712 and have DIFF, DISP, ISP, LAT, MSP.

@johntruckenbrodt
Copy link
Owner

Thanks Caitlin, that's a very good point. These issues with the shared objects are common and it sure would be good to forward any errors to pyroSAR. The parser raises an error when a GAMMA command raises an error but this error is caught in pyroSAR.parser.parse_module. So I guess what happened is that none of the commands in disp could be parsed and thus the whole Python file was not created. You likely didn't notice this during parsing because this function just logs an info message that some commands could not be parsed and displaying logging messages is not turned on by default. See Logging.
I admit this is not really good practice and this kind of information must be made available to the user in any case.

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

2 participants