[RFC,v1] treewide: Use `argparse.FileType` in more places

24328 diff mbox series

Convert some scripts to use `argparse.FileType` where the change is relatively
easily doable. This allows better error messages as e.g. missing input files
will be detected during argument parsing. And it also makes writing to stdout
in absence of an explicit argument simpler.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 Documentation/gen-doxyfile.py       | 16 +++++-----------
 src/py/libcamera/gen-py-controls.py | 19 ++++++++-----------
 src/py/libcamera/gen-py-formats.py  | 26 +++++++-------------------
 utils/codegen/gen-controls.py       | 27 +++++++++++----------------
 utils/codegen/gen-formats.py        | 27 ++++++++++-----------------
 utils/codegen/gen-gst-controls.py   | 18 ++++++++----------
 6 files changed, 49 insertions(+), 84 deletions(-)

Hi Barnabás,

On Thu, Sep 11, 2025 at 05:40:15PM +0200, Barnabás Pőcze wrote:
> Convert some scripts to use `argparse.FileType` where the change is relatively
> easily doable. This allows better error messages as e.g. missing input files
> will be detected during argument parsing. And it also makes writing to stdout
> in absence of an explicit argument simpler.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  Documentation/gen-doxyfile.py       | 16 +++++-----------
>  src/py/libcamera/gen-py-controls.py | 19 ++++++++-----------
>  src/py/libcamera/gen-py-formats.py  | 26 +++++++-------------------
>  utils/codegen/gen-controls.py       | 27 +++++++++++----------------
>  utils/codegen/gen-formats.py        | 27 ++++++++++-----------------
>  utils/codegen/gen-gst-controls.py   | 18 ++++++++----------
>  6 files changed, 49 insertions(+), 84 deletions(-)
> 
> diff --git a/Documentation/gen-doxyfile.py b/Documentation/gen-doxyfile.py
> index c265bc2f3..af9b24aa0 100755
> --- a/Documentation/gen-doxyfile.py
> +++ b/Documentation/gen-doxyfile.py
> @@ -12,15 +12,6 @@ import string
>  import sys
>  
>  
> -def fill_template(template, data):
> -
> -    template = open(template, 'rb').read()
> -    template = template.decode('utf-8')
> -    template = string.Template(template)
> -
> -    return template.substitute(data)
> -
> -
>  def main(argv):
>  
>      parser = argparse.ArgumentParser()
> @@ -28,7 +19,8 @@ def main(argv):
>                          type=argparse.FileType('w', encoding='utf-8'),
>                          default=sys.stdout,
>                          help='Output file name (default: standard output)')
> -    parser.add_argument('template', metavar='doxyfile.tmpl', type=str,
> +    parser.add_argument('template', metavar='doxyfile.tmpl',
> +                        type=argparse.FileType('r', encoding='utf-8'),
>                          help='Doxyfile template')
>      parser.add_argument('inputs', type=str, nargs='*',
>                          help='Input files')
> @@ -36,7 +28,9 @@ def main(argv):
>      args = parser.parse_args(argv[1:])
>  
>      inputs = [f'"{os.path.realpath(input)}"' for input in args.inputs]
> -    data = fill_template(args.template, {'inputs': (' \\\n' + ' ' * 25).join(inputs)})
> +    data = string.Template(args.template.read()).substitute({
> +        'inputs': (' \\\n' + ' ' * 25).join(inputs),
> +    })
>      args.output.write(data)
>  
>      return 0
> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py
> index d43a7c1c7..97849eb34 100755
> --- a/src/py/libcamera/gen-py-controls.py
> +++ b/src/py/libcamera/gen-py-controls.py
> @@ -60,11 +60,13 @@ def main(argv):
>      parser = argparse.ArgumentParser()
>      parser.add_argument('--mode', '-m', type=str, required=True,
>                          help='Mode is either "controls" or "properties"')
> -    parser.add_argument('--output', '-o', metavar='file', type=str,
> +    parser.add_argument('--output', '-o', metavar='file', default=sys.stdout,
> +                        type=argparse.FileType('w', encoding='utf-8'),
>                          help='Output file name. Defaults to standard output if not specified.')
> -    parser.add_argument('--template', '-t', type=str, required=True,
> +    parser.add_argument('--template', '-t', required=True,
> +                        type=argparse.FileType('r', encoding='utf-8'),
>                          help='Template file name.')
> -    parser.add_argument('input', type=str, nargs='+',
> +    parser.add_argument('input', type=argparse.FileType('rb'), nargs='+',
>                          help='Input file name.')
>      args = parser.parse_args(argv[1:])
>  
> @@ -76,7 +78,7 @@ def main(argv):
>      vendors = []
>  
>      for input in args.input:
> -        data = yaml.safe_load(open(input, 'rb').read())
> +        data = yaml.safe_load(input)
>  
>          vendor = data['vendor']
>          if vendor != 'libcamera':
> @@ -94,15 +96,10 @@ def main(argv):
>      }
>  
>      env = jinja2.Environment()
> -    template = env.from_string(open(args.template, 'r', encoding='utf-8').read())
> +    template = env.from_string(args.template.read())
>      string = template.render(data)
>  
> -    if args.output:
> -        output = open(args.output, 'w', encoding='utf-8')
> -        output.write(string)
> -        output.close()
> -    else:
> -        sys.stdout.write(string)
> +    args.output.write(string)
>  
>      return 0
>  
> diff --git a/src/py/libcamera/gen-py-formats.py b/src/py/libcamera/gen-py-formats.py
> index 0ff1d12ac..4d1963333 100755
> --- a/src/py/libcamera/gen-py-formats.py
> +++ b/src/py/libcamera/gen-py-formats.py
> @@ -19,35 +19,23 @@ def generate(formats):
>      return {'formats': '\n'.join(fmts)}
>  
>  
> -def fill_template(template, data):
> -    with open(template, encoding='utf-8') as f:
> -        template = f.read()
> -
> -    template = string.Template(template)
> -    return template.substitute(data)
> -
> -
>  def main(argv):
>      parser = argparse.ArgumentParser()
> -    parser.add_argument('-o', dest='output', metavar='file', type=str,
> +    parser.add_argument('-o', dest='output', metavar='file', default=sys.stdout,
> +                        type=argparse.FileType('w', encoding='utf-8'),
>                          help='Output file name. Defaults to standard output if not specified.')
> -    parser.add_argument('input', type=str,
> +    parser.add_argument('input', type=argparse.FileType('rb'),

Why are you handling this one as binary ? It seems to be a change in
behaviour, if that's on purpose it should be explained in the commit
message. If it's for consistency reasons, I would split that to a
separate commit that aligns all YAML files as binary (if that's the
desired outcome, does yaml.safe_load() perform utf-8 decoding
internally, or do we want to handle it explicitly ?).

>                          help='Input file name.')
> -    parser.add_argument('template', type=str,
> +    parser.add_argument('template', type=argparse.FileType('r', encoding='utf-8'),
>                          help='Template file name.')
>      args = parser.parse_args(argv[1:])
>  
> -    with open(args.input, encoding='utf-8') as f:
> -        formats = yaml.safe_load(f)['formats']
> +    formats = yaml.safe_load(args.input)['formats']
>  
>      data = generate(formats)
> -    data = fill_template(args.template, data)
> +    data = string.Template(args.template.read()).substitute(data)
>  
> -    if args.output:
> -        with open(args.output, 'w', encoding='utf-8') as f:
> -            f.write(data)
> -    else:
> -        sys.stdout.write(data)
> +    args.output.write(data)
>  
>      return 0
>  
> diff --git a/utils/codegen/gen-controls.py b/utils/codegen/gen-controls.py
> index 59b716c1c..90aba344b 100755
> --- a/utils/codegen/gen-controls.py
> +++ b/utils/codegen/gen-controls.py
> @@ -44,25 +44,25 @@ def main(argv):
>      parser = argparse.ArgumentParser()
>      parser.add_argument('--mode', '-m', type=str, required=True, choices=['controls', 'properties'],
>                          help='Mode of operation')
> -    parser.add_argument('--output', '-o', metavar='file', type=str,
> +    parser.add_argument('--output', '-o', metavar='file', default=sys.stdout,
> +                        type=argparse.FileType('w', encoding='utf-8'),
>                          help='Output file name. Defaults to standard output if not specified.')
> -    parser.add_argument('--ranges', '-r', type=str, required=True,
> +    parser.add_argument('--ranges', '-r', required=True, type=argparse.FileType('rb'),
>                          help='Control id range reservation file.')
> -    parser.add_argument('--template', '-t', dest='template', type=str, required=True,
> +    parser.add_argument('--template', '-t', dest='template', required=True,
> +                        type=argparse.FileType('r', encoding='utf-8'),
>                          help='Template file name.')
> -    parser.add_argument('input', type=str, nargs='+',
> +    parser.add_argument('input', nargs='+',
> +                        type=argparse.FileType('rb'),
>                          help='Input file name.')
>  
>      args = parser.parse_args(argv[1:])
>  
> -    ranges = {}
> -    with open(args.ranges, 'rb') as f:
> -        data = open(args.ranges, 'rb').read()
> -        ranges = yaml.safe_load(data)['ranges']
> +    ranges = yaml.safe_load(args.ranges)['ranges']
>  
>      controls = {}
>      for input in args.input:
> -        data = yaml.safe_load(open(input, 'rb').read())
> +        data = yaml.safe_load(input)
>  
>          vendor = data['vendor']
>          if vendor not in ranges.keys():
> @@ -92,15 +92,10 @@ def main(argv):
>      env = jinja2.Environment()
>      env.filters['format_description'] = format_description
>      env.filters['snake_case'] = snake_case
> -    template = env.from_string(open(args.template, 'r', encoding='utf-8').read())
> +    template = env.from_string(args.template.read())
>      string = template.render(data)
>  
> -    if args.output:
> -        output = open(args.output, 'w', encoding='utf-8')
> -        output.write(string)
> -        output.close()
> -    else:
> -        sys.stdout.write(string)
> +    args.output.write(string)
>  
>      return 0
>  
> diff --git a/utils/codegen/gen-formats.py b/utils/codegen/gen-formats.py
> index 7542d8841..411daeabc 100755
> --- a/utils/codegen/gen-formats.py
> +++ b/utils/codegen/gen-formats.py
> @@ -19,14 +19,12 @@ class DRMFourCC(object):
>      mod_vendor_regex = re.compile(r"#define DRM_FORMAT_MOD_VENDOR_([A-Z0-9_]+)[ \t]+([0-9a-fA-Fx]+)")
>      mod_regex = re.compile(r"#define ([A-Za-z0-9_]+)[ \t]+fourcc_mod_code\(([A-Z0-9_]+), ([0-9a-fA-Fx]+)\)")
>  
> -    def __init__(self, filename):
> +    def __init__(self, lines):

I would name the parameter 'file', 'lines' makes me think it's a list or
tuple of strings.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>          self.formats = {}
>          self.vendors = {}
>          self.mods = {}
>  
> -        for line in open(filename, 'rb').readlines():
> -            line = line.decode('utf-8')
> -
> +        for line in lines:
>              match = DRMFourCC.format_regex.match(line)
>              if match:
>                  format, fourcc = match.groups()
> @@ -80,32 +78,27 @@ def main(argv):
>  
>      # Parse command line arguments
>      parser = argparse.ArgumentParser()
> -    parser.add_argument('-o', dest='output', metavar='file', type=str,
> +    parser.add_argument('-o', dest='output', metavar='file', default=sys.stdout,
> +                        type=argparse.FileType('w', encoding='utf-8'),
>                          help='Output file name. Defaults to standard output if not specified.')
> -    parser.add_argument('input', type=str,
> +    parser.add_argument('input', type=argparse.FileType('rb'),
>                          help='Input file name.')
> -    parser.add_argument('template', type=str,
> +    parser.add_argument('template', type=argparse.FileType('r', encoding='utf-8'),
>                          help='Template file name.')
> -    parser.add_argument('drm_fourcc', type=str,
> +    parser.add_argument('drm_fourcc', type=argparse.FileType('r', encoding='utf-8'),
>                          help='Path to drm_fourcc.h.')
>      args = parser.parse_args(argv[1:])
>  
> -    data = open(args.input, 'rb').read()
> -    formats = yaml.safe_load(data)['formats']
> +    formats = yaml.safe_load(args.input)['formats']
>      drm_fourcc = DRMFourCC(args.drm_fourcc)
>  
>      env = jinja2.Environment()
> -    template = env.from_string(open(args.template, 'r', encoding='utf-8').read())
> +    template = env.from_string(args.template.read())
>      string = template.render({
>          'formats': generate_formats(formats, drm_fourcc),
>      })
>  
> -    if args.output:
> -        output = open(args.output, 'wb')
> -        output.write(string.encode('utf-8'))
> -        output.close()
> -    else:
> -        sys.stdout.write(string)
> +    args.output.write(string)
>  
>      return 0
>  
> diff --git a/utils/codegen/gen-gst-controls.py b/utils/codegen/gen-gst-controls.py
> index 4ca76049e..31f18625f 100755
> --- a/utils/codegen/gen-gst-controls.py
> +++ b/utils/codegen/gen-gst-controls.py
> @@ -138,18 +138,20 @@ def extend_control(ctrl):
>  def main(argv):
>      # Parse command line arguments
>      parser = argparse.ArgumentParser()
> -    parser.add_argument('--output', '-o', metavar='file', type=str,
> +    parser.add_argument('--output', '-o', metavar='file', default=sys.stdout,
> +                        type=argparse.FileType('w', encoding='utf-8'),
>                          help='Output file name. Defaults to standard output if not specified.')
> -    parser.add_argument('--template', '-t', dest='template', type=str, required=True,
> +    parser.add_argument('--template', '-t', dest='template', required=True,
> +                        type=argparse.FileType('r', encoding='utf-8'),
>                          help='Template file name.')
> -    parser.add_argument('input', type=str, nargs='+',
> +    parser.add_argument('input', nargs='+', type=argparse.FileType('rb'),
>                          help='Input file name.')
>  
>      args = parser.parse_args(argv[1:])
>  
>      controls = {}
>      for input in args.input:
> -        data = yaml.safe_load(open(input, 'rb').read())
> +        data = yaml.safe_load(input)
>  
>          vendor = data['vendor']
>          ctrls = controls.setdefault(vendor, [])
> @@ -167,14 +169,10 @@ def main(argv):
>      env.filters['indent_str'] = indent_str
>      env.filters['snake_case'] = snake_case
>      env.filters['kebab_case'] = kebab_case
> -    template = env.from_string(open(args.template, 'r', encoding='utf-8').read())
> +    template = env.from_string(args.template.read())
>      string = template.render(data)
>  
> -    if args.output:
> -        with open(args.output, 'w', encoding='utf-8') as output:
> -            output.write(string)
> -    else:
> -        sys.stdout.write(string)
> +    args.output.write(string)
>  
>      return 0
>