Skip to content

Improve error handling on importing modules for GAMMA #325

Open
@caitlinadams

Description

@caitlinadams

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

Activity

johntruckenbrodt

johntruckenbrodt commented on Dec 11, 2024

@johntruckenbrodt
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

caitlinadams commented on Dec 12, 2024

@caitlinadams
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

johntruckenbrodt commented on Dec 12, 2024

@johntruckenbrodt
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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Improve error handling on importing modules for GAMMA · Issue #325 · johntruckenbrodt/pyroSAR