Stop exam cheating using python: Part 2 - Argparse and config

Question

Intro

This is a spiritual successor to the question Stop cheating on home exams using python

The details are not important, but I decided to reimplement everything using dataclasses. My script is now working, but is a bit big (around 1200 lines). So in order to ease the burden I will split my codereview into a few questions spaced over a few days or weeks.

This question only focuses on how I store and use the configuration options provided by the user

My filestructure is as follows

uni.py <- mainfile
candidates.py
exam.py
texfile.py
variable.py
problem.py
tools.py
default_config.py

Now the defaults of the program is stored in a config.py file which looks like this

from default_config import Config

CONFIG: Config = {
    "texfile": "None",
    "candidates": 0,
    "set_variable": "setfpvar",
    "get_variable": "fv",
    "input_formats": ["input", "include"],
    "latex_comment": "%",
    "PDFLATEX_OR_XELATEX": "xelatex",
    "equal_comment_indent_per_problem": True,
    "python_code_as_comment": True,
    "use_python_in_comment": False,
    "candidate_number_pad": 3,
    "subdir": True,
    "all_subdirs_equal": True,
    "tex_dir": "tex",
    "pdf_dir": "pdf",
    "solution_dir": "solution",
    "build_dir": "aux"
}

Every file that needs it, imports this file to read of the default values. For ease of readability I decided that the user interacts with this config file using a config.yaml file that is identical, but with added comments.

config:
  texfile: None
  candidates: 0

  # Sets the names of misc commands used in the texfile
  set_variable: setfpvar
  get_variable: fv
  input_formats:
  - input
  - include
  latex_comment: '%'

  # Decides which engine to use to compile the texfile
  # Standard options are 'xelatex' or 'pdflatex'
  PDFLATEX_OR_XELATEX: xelatex

  # If comment is true the python code to generate the
  # variable's value is placed as a comment next to it
  # in the tex file
  equal_comment_indent_per_problem: true
  python_code_as_comment: true

  # If true the comment mentined above can be used in reverse;
  # meaning if a comment is present next to a varible we try
  # to run it as a Python command to generate the value. 
  # DANGEROUS
  use_python_in_comment: false

  # Number of places to pad the candidate number (001)=3
  candidate_number_pad: 3

  # Defines which subdirectories to put each file in
  # if subdir is false everything is dumped into main
  subdir: true
  all_subdirs_equal: true
  tex_dir: tex
  pdf_dir: pdf
  solution_dir: solution
  build_dir: aux

The idea is that command line arguments succeed the config options, which again succeeds the default config options. The following code below has two jobs it either populates the config.py file, or it creates a config.yaml file for the user to play around with.

• I was very happy with being able to do from config import CONFIG in all of my classes, which was a major reason why I decided to implement the config system in this manner.

This is just the parser, as mentioned I have not included the exam generation. Run as

uni.py exam -h or uni.py config -h

To see the possible options.

uni.py

import argparse
from pathlib import Path
from typing import Union

from default_config import (
    dump_config_2_yaml,
    load_default_config,
    load_yaml_from_config,
    dump_config_2_python,
)


def is_float(potential_float: str) -> bool:
    try:
        float(potential_float)
        return True
    except ValueError:
        return False


# Refference
# Title: In Python, using argparse, allow only positive integers
# Source: https://stackoverflow.com/a/14117511/1048781
def non_negative_int(value: str):
    """Raises an exception unless value is non-negative

    Args:
        param1: A value parsed as a string

    Returns:
        The input string value as an integer.

    Raises:
        ArgumentTypeError: uni.py: error: argument --candidates/-n: Expected a
        positive int for number of candidates, got
    """
    if not is_float(value):
        raise argparse.ArgumentTypeError(
            f"Expected a positive int for number of candidates, got {value}"
        )

    negative, not_integer = (ivalue := float(value)) < 0, int(ivalue) != ivalue
    if negative or not_integer:
        raise argparse.ArgumentTypeError(
            f"Expected a positive int for number of candidates, got {value}"
        )
    return ivalue


def check_dir(path: Union[Path, str]):
    if (dir_ := Path(path)).is_dir():
        return dir_.resolve()
    raise argparse.ArgumentTypeError(
        f"Expected a directory, but got '{path}' instead, which does not exist"
    )


def check_path(path: Path, suffix: str) -> Path:
    if not path.is_file():
        raise argparse.ArgumentTypeError(
            "Expected a .{suffix} file, but the path provided does not exist"
        )
    elif not path.suffix == "." + suffix:
        raise argparse.ArgumentTypeError(
            f"Expected a .{suffix} file, got a {path.suffix} instead"
        )
    return path


def texfile(path: str) -> Path:
    """Check if the provided path is an existing tex file"""
    return check_path(Path(path), "tex")


def yamlfile(path: str) -> Path:
    """Check if the provided path is an existing yaml file"""
    return check_path(Path(path), "yaml")


def yaml_or_texfile(path: str) -> Path:
    suffixes = [".tex", ".yaml"]
    if not (path_ := Path(path)).is_file():
        raise argparse.ArgumentTypeError(
            "Expected one of {suffixes} type file,"
            + " but the path provided does not exist"
        )
    elif path_.suffix not in suffixes:
        raise argparse.ArgumentTypeError(
            "Expected one of {suffixes} type file, "
            + f"but got a {path_.suffix} instead"
        )
    return path_


def yaml_or_dir(path: str) -> Path:
    suffixes = [".yaml"]
    if (path_ := Path(path)).is_dir():
        return path_.resolve()
    if path_.suffix in suffixes:
        return path_
    raise argparse.ArgumentTypeError(
        "Expected a '.yaml' type filename or an "
        + f"existing dir, got a {path_} type instead"
    )


def new_parser():
    # create the top-level parser
    parser = argparse.ArgumentParser(prog="PROG")
    subparsers = parser.add_subparsers(
        help="command help", dest="commands", required=True
    )

    # =================================================================================
    #    uni exam
    # =================================================================================
    parsers_exam = subparsers.add_parser(
        "exam", help="Generate random variants from a texfile"
    )
    parsers_exam.add_argument(
        "path",
        type=yaml_or_texfile,
        metavar="PATH",
        help="The exam.tex file OR a config.yaml file (use 'uni generate"
        + "config' to generate a default config file",
    )
    parsers_exam.add_argument(
        "--texfile",
        type=texfile,
        metavar="PATH",
        help="The exam.tex (Useful if you set a config file using PATH"
        + " , and want to override the texfile in it)",
    )
    parsers_exam.add_argument(
        "--candidates",
        "-n",
        type=non_negative_int,
        help="Number of candidates (Exams to generate)",
    )
    parsers_exam.add_argument(
        "--subdirs-equal", action=argparse.BooleanOptionalAction, default=None
    )
    parsers_exam.add_argument(
        "--texdir",
        type=str,
        help="Sets the directory to place the tex files",
    )
    parsers_exam.add_argument(
        "--pdfdir",
        type=str,
        help="Sets the directory to place the pdf files",
    )
    parsers_exam.add_argument(
        "--solutiondir",
        type=str,
        help="Sets the directory to place the solutions files",
    )
    parsers_exam.add_argument(
        "--builddir",
        type=str,
        help="Sets the directory to place the build files",
    )
    parsers_exam.add_argument(
        "--save-config",
        type=yaml_or_dir,
        metavar="PATH",
        nargs="?",
        const=Path().cwd(),
        default=None,
        help="Save the current settings."
        + " Default path is current working directory",
    )
    # =================================================================================
    #    uni config
    # =================================================================================
    parser_default_config = subparsers.add_parser(
        "config", help="Use to generate the default config file in PATH"
    )
    parser_default_config.add_argument(
        "path",
        type=yaml_or_dir,
        metavar="PATH",
        nargs="?",
        default=Path().cwd(),
        help="Path (filename or dir) to generate the"
        + " default config file for exams",
    )
    return parser


ARGPARSE_2_CONFIG = {
    "texfile": "texfile",
    "candidates": "candidates",
    "subdirs_equal": "subdirs-equal",
    "texdir": "texdir",
    "solutiondir": "solutiondir",
    "builddir": "builddir",
    "pdfdir": "pdfdir",
}


def generate_config(args):
    if args.path.suffix == ".yaml":
        config = load_yaml_from_config(args.path)
    else:
        config = load_default_config()
        # The default config has no texfile by default
        config["config"]["texfile"] = args.path

    # Command line arguments takes president over config.yaml
    for argparse_name, config_name in ARGPARSE_2_CONFIG.items():
        if (argument := getattr(args, argparse_name)) is not None:
            config["config"][config_name] = argument

    return config


def exam(args):

    config = generate_config(args)

    # The user may have written a badly formated value for number of candidates
    non_negative_int(config["config"]["candidates"])

    if config["config"]["texfile"] is None:
        raise argparse.ArgumentTypeError(
            "PROG exam: error: the following arguments"
            + " are required: PATH, --candidates/-n"
        )

    if args.save_config is not None:
        if args.save_config.is_dir():
            directory = args.save_config
            name = "config.yaml"
        else:
            directory = args.save_config.parent
            name = args.save_config.name
        dump_config_2_yaml(config=config, path=directory / name)

    dump_config_2_python(config)


def config(args):
    if args.path.suffix == ".yaml":
        config_path = args.path
    else:
        directory = args.path
        config_path = directory / "config.yaml"
    if dump_config_2_yaml(config=load_default_config(), path=config_path):
        print(f"'{config_path}' successfully created")


if __name__ == "__main__":
    parser = new_parser()
    args = parser.parse_args()

    if args.commands == "exam":
        exam(args)
    elif args.commands == "config":
        config(args)

default_config.py

import pprint
from pathlib import Path

import ruamel.yaml
from typing import TypedDict

yaml = ruamel.yaml.YAML()
yaml.preserve_quotes = True

DEFAULT_YAML = """\
config:
  
  texfile: None
  candidates: 0

  # Sets the names of misc commands used in the texfile
  set_variable: setfpvar
  get_variable: fv
  input_formats: 
    - input
    - include
  latex_comment: '%'

  # Decides which engine to use to compile the texfile
  # Standard options are 'xelatex' or 'pdflatex'
  PDFLATEX_OR_XELATEX: xelatex

  # If comment is true the python code to generate the
  # variable's value is placed as a comment next to it
  # in the tex file
  equal_comment_indent_per_problem: True
  python_code_as_comment: True

  # If true the comment mentined above can be used in reverse;
  # meaning if a comment is present next to a varible we try
  # to run it as a Python command to generate the value. 
  # DANGEROUS
  use_python_in_comment: False

  # Number of places to pad the candidate number (001)=3
  candidate_number_pad: 3

  # Defines which subdirectories to put each file in
  # if subdir is false everything is dumped into main
  subdir: True
  all_subdirs_equal: True
  tex_dir: tex
  pdf_dir: pdf
  solution_dir: solution
  build_dir: aux
"""


class Config(TypedDict):
    texfile: str
    candidates: int
    set_variable: str
    get_variable: str
    input_formats: list[str]
    latex_comment: str
    PDFLATEX_OR_XELATEX: str
    equal_comment_indent_per_problem: bool
    python_code_as_comment: bool
    use_python_in_comment: bool
    candidate_number_pad: int
    subdir: bool
    all_subdirs_equal: bool
    tex_dir: str
    pdf_dir: str
    solution_dir: str
    build_dir: str


PATH = Path(__file__)
DIR = PATH.parent
YAML_CONFIG_PATH = DIR / PATH.with_suffix(".yaml")
PYTHON_CONFIG_PATH = DIR / "config.py"

if not YAML_CONFIG_PATH.is_file():
    yaml.dump(yaml.load(DEFAULT_YAML), YAML_CONFIG_PATH)


def load_yaml_from_config(path):
    return yaml.load(path)


def dump_config_2_yaml(config, path):
    if not path.is_file():
        yaml.dump(config, path)
        return True
    print(
        "Ooops, it looks like the file \n\n"
        + str(path)
        + "\n\n already exists. Do you want to override it?"
    )
    if input("[y/N]").lower()[0] == "y":
        yaml.dump(config, path)
        return True
    return False


def load_default_config(path=YAML_CONFIG_PATH):
    if not path.is_file():
        yaml.dump(yaml.load(path), YAML_CONFIG_PATH)
    return yaml.load(path)


def dump_config_2_python(config, path=PYTHON_CONFIG_PATH, indent=4):
    imports = "from default_config import Config"
    config_body = pprint.pformat(config["config"], indent=indent, sort_dicts=False)[
        1:-1
    ]
    with open(path, "w") as f:
        f.writelines(
            imports
            + "\n\nCONFIG: Config = {\n "
            + config_body.replace("'", '"')
            + "\n}"
        )

Answer 1

Your uni.py is your "main file"; fine. As such, it should have a #!/usr/bin/env python3 shebang. The nicer option is to make a proper module directory tree containing a __main__.py such that running python -m uni runs your module's main entry point.

It's a good idea to ship a default config.yaml file inside of your module; this will mean that all of your DEFAULT_YAML = """ should simply be deleted.

is_float is, first of all, not actually what you care to evaluate; you care specifically about the narrower case of integers. Secondly, the way it's structured now prevents you from being able to raise with an inner exception cause object using a from clause. Also, you don't care about positive integers; you care about non-negative integers, which include 0. I would instead expect:

def non_negative_int(value: str, name: str) -> int:
    """Raises an exception unless value is non-negative"""

    try:
        ivalue = int(value)
    except ValueError as e:
        raise argparse.ArgumentTypeError(
            f"Expected an integer for {name}, got {value}"
        ) from e

    if ivalue < 0:
        raise argparse.ArgumentTypeError(
            f"Expected a non-negative int for {name}, got {ivalue}"
        )

    return ivalue

Also note that this should not be hard-coded for number of candidates and should just accept a name.

You're over-using the walrus operator. It's useful for the inside of a generator when you really, really need variables in-line. I've not found any of your cases that benefit from it, and you should just use traditional variable assignment instead.

You've started some type hinting (good!) but it's incomplete. For instance, check_dir is missing its return hint.

Some of your error messages don't make sense, like

if not path.is_file():
    raise argparse.ArgumentTypeError(
        "Expected a .{suffix} file, but the path provided does not exist"
    )

It doesn't matter what suffix you expect; all that matters is that the provided path doesn't exist but you don't actually tell the user what that was. Instead, just say f'{path} does not exist'.

Avoid passing around two different suffix formats, one with a . and one without. Just use .suffix everywhere since that's what pathlib expects; and remove "." + suffix.

texfile and yamlfile are a little awkward. They have two responsibilities - convert a string to a Path, and validate that path. check_path should, then, not return anything, and the first two functions should do the path construction, pass the path to check_path, and then return it.

suffixes should be a set literal {} instead of a list [].

Some of your string interpolation is incorrect. For example, "Expected one of {suffixes} type file," is missing an f prefix.

You don't need + string concatenation in many of the places you've currently written it, and can use implicit literal concatenation instead - for example,

    raise argparse.ArgumentTypeError(
        "Expected one of {suffixes} type file,"
        + " but the path provided does not exist"
    )

can become

    raise argparse.ArgumentTypeError(
        f"Expected one of {suffixes} type file, "
        "but the path provided does not exist"
    )

Whenever you have the temptation to write a "chapter-heading comment" like

# =================================================================================
#    uni exam
# =================================================================================

that's a very good indication that you should just make a subroutine. It will be self-documenting, you can delete the big comment header, and it will be more testable and maintainable.

Some of your documentation can use a little work; for instance there's a parenthesis imbalance here:

    help="The exam.tex file OR a config.yaml file (use 'uni generate"
    + "config' to generate a default config file",

When you want to call cwd like Path().cwd(), it's a static function. Don't construct a Path; just call it on the type like Path.cwd().

It's good that you have a __main__ check. However, the symbols on the inside of that if are still in global scope. Make a main() to contain that code.

When checking your args.comments, you should add a sanity else: at the end that raises if the command is unrecognized.

The initialization of these globals:

PATH = Path(__file__)
DIR = PATH.parent
YAML_CONFIG_PATH = DIR / PATH.with_suffix(".yaml")
PYTHON_CONFIG_PATH = DIR / "config.py"

if not YAML_CONFIG_PATH.is_file():
    yaml.dump(yaml.load(DEFAULT_YAML), YAML_CONFIG_PATH)

should be captured in a function:

def make_default_paths() -> Tuple[Path, Path]:
    path = Path(__file__)
    dir = path.parent
    yaml_config_path = dir / path.with_suffix(".yaml")
    python_config_path = dir / "config.py"

    if not yaml_config_path.is_file():
        yaml.dump(yaml.load(DEFAULT_YAML), yaml_config_path)

    return yaml_config_path, python_config_path


YAML_CONFIG_PATH, PYTHON_CONFIG_PATH = make_default_paths()

Your if input("[y/N]").lower()[0] == "y": is fragile. The [0] is going to raise if the user did what you told them and pressed enter to use the default of "no". Instead, use startswith which will still work with a blank string.

writelines is not what you want; just call write - you have a single string, not an iterable of strings.