Importing fmask changes gdal exception setting

[gillins]

Original report by Christof Kaufmann (Bitbucket: ChristofKaufmann, GitHub: ChristofKaufmann).

---

Dear all,

this is rather a discussion than a report. When working with python-fmask I noticed that importing parts of it change the GDAL exception setting. So the following - maybe unusual - example fails for me:

from fmask import landsatTOA

# Setting the exceptions off (default) make gdal_merge work
#from osgeo import gdal
#gdal.DontUseExceptions()

import sys
sys.path.append('/usr/bin')
import gdal_merge
gdal_merge.main(['gdal_merge.py', '-separate', '-o', 'test.tif', 'band1.tif', 'band2.tif'])

This should actually stack band1.tif and band2.tif into a single file test.tif. However, gdal_merge assumes that exceptions are turned off (which is by itself an issue maybe). I can work around this, by using

import contextlib
from osgeo import gdal

@contextlib.contextmanager
def gdal_exceptions(state=None):
    orig_state = gdal.GetUseExceptions()
    if state is not None:
        if state:
            gdal.UseExceptions()
        else:
            gdal.DontUseExceptions()
    yield
    if orig_state:
        gdal.UseExceptions()
    else:
        gdal.DontUseExceptions()

and then I can surround the imports of fmask like:

with gdal_exceptions(): # preserves the original exception state
    from fmask import landsatTOA

and calls to fmask functions like:

with gdal_exceptions(True):  # set exception state temporarily True
    landsatTOA.makeTOAReflectance(...)

But it is just a work around.

So a possible solution could be to wrap gdal_merge in a subprocess, but I don't really like spawning multiple processes. Another possible solution would be for python-fmask to not change the exception setting. However, I tried to start a pull request with a similar solution as gdal_exceptions, but if the exceptions are really important for the python-fmask code, there would be way too many with statements required in python-fmask. What is your opinion on this?

BTW: I really import the fmask command line scripts and use them similarly as gdal_merge in the example above. I just simplified the problem here as much as possible.

[gillins]

Original comment by Sam Gillingham (Bitbucket: gillins, GitHub: gillins).

---

Hi there,

Setting the GDAL exceptions was done in response to issues like #9 where the user was getting some error reading a file but nothing was being reported to the user.

The whole GDAL exceptions thing is very weird, and I'm not sure there is a correct solution. Your workaround makes sense here. @neilflood may have some thoughts?

Sam.

[gillins]

Original comment by Sam Gillingham (Bitbucket: gillins, GitHub: gillins).

---

Hi @ChristofKaufmann, @neilflood,

I had a go at creating a decorator that preserves state in the ex_decorator branch. However, I then discovered that rios also changes the exception state when you import it....

We either leave this as it is, or modify rios also.

Sam.

[gillins]

Original comment by Neil Flood (Bitbucket: neilflood, GitHub: neilflood).

---

Hi Christof and Sam,

thanks for having a go with that. I would definitely be against also modifying rios to match this sort of thing.

Christof - I think that the approach you are using seems a bit strange. Would it not be simpler just to use the usual subprocess mechanisms to run the commands separately? You seem to be running on Linux, so there is no particular cost to doing that.

Neil

[gillins]

Original comment by Christof Kaufmann (Bitbucket: ChristofKaufmann, GitHub: ChristofKaufmann).

---

Hi Neil,

Yes, I am using Linux, but the script I am writing should also work on Windows. Though subprocess calls can work in Windows, too. Indeed I used subprocess calls, but switched then to this approach, because I thought it would be cleaner to run python scripts as function calls. Now I am not sure anymore. It both seems ugly in a way. So, I guess, I leave it as it is, since it works and there is no strong reason to change back.

Anyway, thanks for the discussion. From my side the issue can be closed.

Offtopic: Maybe you can add in the documentation on the main page in the section USGS Landsat that the included quality layer pixel_qa, which uses a C implementation of the FMASK algorithm. Could be interesting for some people reading that section.

Best regards,
Christof

[gillins]

Original comment by Neil Flood (Bitbucket: neilflood, GitHub: neilflood).

---

Hi Christof,

OK, thank, I will close it off. I agree, a good discussion, even though the ugliness remains.

That's a good point about the USGS Landsat now including Fmask. Obviously this implementation was from long before that was available, but you are right, we should mention that it is now no longer really necessary, since they supply it anyway. For those processing pre-Collection 1 files, the code remains useful, though. I will write a few words around this.

thanks for your thoughts Neil

[gillins]

Original comment by Neil Flood (Bitbucket: neilflood, GitHub: neilflood).

---

The issue seems difficult to resolve without creating further ugliness, so we are leaving it for now.