Standardize gdal-utils scripts return codes

[maphew]

While building the autotests for pyscripts/gdal-utils one of the things that tripped me up for a bit was that the utils do not return the same status code for "was run without arguments". Some are 0 and others are -1. It would be good for the same code meant the same thing across all the scripts in the package.

At the moment I'm only thinking as far as 'no arguments'. I don't know if there are other codes worth trying to standardize on. "input not found" comes to mind, perhaps.

I'm willing to do the work, but do others think it's a good idea too?

cc @idanmiara, @rouault

[rouault]

I'm willing to do the work, but do others think it's a good idea too?

sounds good. I had written initially "Any non-zero value is appropriate", but things seem more complex
But just trying "pip", I see it returns 0. Same for "conda" or "rio"
However "pip install" or "conda install" or "rio info" returns non-zero, as well as "docker-compose"
It looks like most GDAL utilities are in the later category (they are not "meta" utilities like pip/conda/rio) and require at least one input file, so on conclusion, I believe a non-zero value is appropriate.

[maphew]

I've converted all the scripts that were using -1 to 1. However when I started looking at the ones returning 2 it became less clear what to do. Excepting gdal2tiles all of them are using GDALArgumentParser and I don't see where the return value is being set. Thinking this might mean the mainline utils might be using 2 for no args I checked gdal_translate and gdalwarp, but no, both of those use 1.

The 2 scripts:

2 osgeo_utils\gdal2xyz.py
2 osgeo_utils\gdal_calc.py
2 osgeo_utils\gdal_fillnodata.py
2 osgeo_utils\gdal_polygonize.py
2 osgeo_utils\pct2rgb.py
2 osgeo_utils\rgb2pct.py
2 osgeo_utils\samples\gdallocationinfo.py

Stopping here for the moment.

[maphew]

Most of the return 0 scripts are in auxillary, which by the look of them are meant to be loaded as modules and not run as scripts, so I'm leaving them alone.

The exceptions are:

0 osgeo_utils\samples\gdal_create_pdf.py
0 osgeo_utils\samples\mkgraticule.py

Make graticule could be considered broken, in that without arguments it just goes ahead and does work and passing -h or broken arguments is required to get the help message. Opinions could differ here as it is in the 'samples' folder after all.

Create pdf script has me confused. I haven't been able to figure out where the return code is coming from at all. For instance this change in 1a34f9e still returns 0:

def Usage():
    print('Usage: gdal_create_pdf composition.xml out.pdf')
    return 99

Seeking to understand what's happening I looked at other samples which use gdal.GeneralCmdLineProcessor(argv) such as gdal_ls.py, and there changing the def Usage(): return value works as expected.

Sign me, scratching head.

[EvertEt]

Looking at gdal_create_pdf.py, it just looks like a missing return at https://github.com/OSGeo/gdal/blob/master/swig/python/gdal-utils/osgeo_utils/samples/gdal_create_pdf.py#L71.

[maphew]

Good catch @EvertEt, thank you! gdal_create_pdf.py is fixed.
I made output file for mkgraticule.py a required parameter.

Now I'll turn to studying how GDALArgumentParser sets it return value.

[maphew]

I couldn't figure out the arg parser class so I embarked on making everything else use 2 instead. I'm questioning the wisdom of that at the moment since so many files are beng touched. However the process has forced me to look more closely at the many scripts and start to internalize the various patterns they use. This has been worthwhile even if the approach might get abandoned. The effort is in branch patch-5561-ret2

[idanmiara]

I can help with the arg parser - please write your questions/issues.

…

On Sat, 9 Apr 2022, 00:55 matt wilkie, ***@***.***> wrote: I couldn't figure out the arg parser class so I embarked on making everything else use 2 instead. I'm questioning the wisdom of that at the moment since so many files are beng touched. However the process has forced me to look more closely at the many scripts and start to internalize the various patterns they use. This has been worthwhile even if the approach might get abandoned. The effort is in branch patch-5561-ret2 <https://github.com/maphew/gdal/tree/patch-5561-ret2> — Reply to this email directly, view it on GitHub <#5561 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGJBBLO5EWUAZJZTEAI7MQLVEC2NHANCNFSM5SJG5TBA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[maphew]

thanks @idanmiara. Here is the path I'm following.

With file = pc2rgb.py:

        r = subprocess.run([sys.executable,
            file],
            shell=True,
            capture_output=True,
            text=True,
            )
        if debug:
            print(f'returncode: {r.returncode}')

I get:

returncode: 2

Digging in to pct2rgb.py I see main() calling PCT2RGB class, calling pct2rgb function, which should error out with an exception.:

def main(argv=sys.argv):
    return PCT2RGB().main(argv)

    def doit(self, **kwargs):
        return pct2rgb(**kwargs)

def doit(**kwargs):
    try:
        ds = pct2rgb(**kwargs)
        return ds, 0
    except:
        return None, 1

def pct2rgb(src_filename: PathLikeOrStr, pct_filename: Optional[PathLikeOrStr], dst_filename: PathLikeOrStr,
            band_number: int = 1, out_bands: int = 3, driver_name: Optional[str] = None):
    # Open source file
    src_ds = open_ds(src_filename)
    if src_ds is None:
        raise Exception(f'Unable to open {src_filename} ')
    ...

So I'm thinking that while PCT2RGB class is invoking GDALArgumentParser it's exiting early somehow with this 2 return code.

def get_parser(self, argv) -> GDALArgumentParser:
    ...
    return parser

I look at auxillary/gdal_argparse.py: class GDALArgumentParser(argparse.ArgumentParser) and I see... well a bunch of stuff I only apprehend the shadowy outlines of. Looking at the return statements doesn't add light. (It doesn't help that I don't understand how classes work. There's this thing called super() but it's not defined anywhere. Shouldn't that cause an error? Oh it's from stdlib builtins.pyi. What the heck is a pyi? "...It is pitch black. You are likely to be eaten by a grue.")

» findstr "return" ..\auxiliary\gdal_argparse.py
        return super().parse_args(args=args, **kwargs)
        return shlex.split(arg_line, comments=True)
        return self._parser
        return kwargs
        return kwargs
            return 0
            return 1
        return epilog or None

So my question is: where is returncode: 2 coming from?

[maphew]

So my question is: where is returncode: 2 coming from?

This was resolved on the mailing list, with the decision that 2 is possibly more comon/standard than 1 for "no parameters supplied". It is the default for argparse library at any rate, so I'm going to use 2 too.

Thread: https://www.mail-archive.com/gdal-dev@lists.osgeo.org/msg37730.html