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}"
)
1 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.
The user may have written a badly formated value for number of candidates
<- I did read your previous question, but just to clarify, your students will not be users of the script, it's just you, right? My reason for asking is because if your students will be using this script, that will influence to what extent reviewers should comment on how user-friendly the script should be \$\endgroup\$