I'm using argparse.ArgumentParser
extensively; however, it comes with a lot of boilerplate to set up, and this is especially noticeable when you've got more than a few common arguments that probably exist between functionally different, but effectively the "same" update scripts.
In detail, I am writing a common update_utils
library, that I use in (currently two) two python scripts. The purpose of this library is kind of generic and perhaps all over the place; it provides a common, sane framework as a "base" to work off without the need to duplicate alot of code between modules.
More specifically, three core features are provided by this module:
- An ABC
Manager
, meant to help with the workflow of parsing the releases of a Git repository, only GitHub right now, which is supposed to be subclassed and used - An ABC
Distribution
, which is subclassed several times in theupdate_utils
library, in order to provide a couple of methods to help with managing system-level packages, and notifying the user as such - An ArgumentParser "default" instance, that should resolve a few issues with boilerplate I've had (mentioned above) when using that particular python module.
class Distribution(ABC):
# FIXME add documentation
@abstractmethod
def check(self, packages: list[Package]) -> list[Package]:
# FIXME add documentation
raise NotImplementedError
@abstractmethod
def install(self, packages: list[Package]) -> tuple[bool, str, str]:
# FIXME add documentation
raise NotImplementedError
NOTE: excuse the lack of implementation, the second bullet point as a concept is something I've started working on very recently, and I want to resolve some design issues while it's at it's infancy, rather than later. I promise this is code from a real project, this section's just not finished yet. To be honest, I'm facing issues here as well, and it'll probably be part of a different question all by itself.
This question is posted in regards to the bullet point 3.
import argparse as ap
DEFAULT_FLAGS = {
"--version": {
"help": "Specify a version to install. Default is latest.",
"required": False,
"default": None
},
"--keep": {
"help": "Specify if downloaded files will be kept after finishing.",
"required": False,
"default": False,
"action": "store_true"
},
"--temporary": {
"help": "Specify temporary directory files will be downloaded at. Default is /tmp/",
"required": False,
"default": None,
"type": str
},
"--destination": {
"help": "Specify installation directory.",
"required": False,
"default": None,
"type": str
},
"--unsafe": {
"help": "Specify if file verification will be skipped. Set by default if unsupported by the repository.",
"required": False,
"default": False,
"action": "store_true"
},
}
def get_default_argparser(description:
p = ap.ArgumentParser(description=description)
for argname, argopts in DEFAULT_FLAGS.items():
p.add_argument(argname, **argopts)
return p
More specifically, it turns out another module that depends on this library, and the "default" implementation of agrparser, while 4 out of 5 flags (seen above) fit the glove and all, it turns out that during the implementation of a specific feature in a dependent module, the --temporary
flag needs to support more complex behaviour rather than the one provided from the get_default_argparser
function.
Specifically, it requires a mutually exclusive group, with a few different options, here's the relevant excerpt from the dependent module:
import update_utils as ut
import os
STEAM_INSTALL_DIR = os.path.expanduser(
"~/.local/share/Steam/compatibilitytools.d/"
)
STEAM_FLATPAK_INSTALL_DIR = os.path.expanduser(
"~/.var/app/com.valvesoftware.Valve/.local/share/Steam/compatibilitytools.d/"
)
LUTRIS_INSTALL_DIR = os.path.expanduser(
"~/.local/share/lutris/runners/wine/"
)
LUTRIS_FLATPAK_INSTALL_DIR = os.path.expanduser(
"~/.var/app/net.lutris.Lutris/data/lutris/runners/wine/"
)
def create_argparser():
p = ut.get_default_argparser(
"Download & extract latest version of the most popular game compatibility layers.",
destination=False
)
destination_group = p.add_mutually_exclusive_group(required=True)
destination_group.add_argument(
"--destination",
help="Install @ custom installation directory",
required=False,
default=None,
type=str
)
destination_group.add_argument(
"--steam",
help=f"Install @ default steam directory\n\t{STEAM_INSTALL_DIR}",
required=False,
default=None,
action="store_true"
)
destination_group.add_argument(
"--steam-flatpak",
help=f"Install @ default steam (flatpak) directory\n\t{STEAM_FLATPAK_INSTALL_DIR}",
required=False,
default=None,
action="store_true"
)
destination_group.add_argument(
"--lutris",
help=f"Install @ default lutris directory\n\t{LUTRIS_INSTALL_DIR}",
required=False,
default=None,
action="store_true"
)
destination_group.add_argument(
"--lutris-flatpak",
help=f"Install @ default lutris (flatpak) directory\n\t{LUTRIS_FLATPAK_INSTALL_DIR}",
required=False,
default=None,
action="store_true"
)
return p
Of course, if I tried to get the argparse
implementation that included --destination
without removing it from the DEFAULT_FLAGS
beforehand, it would result in erroneous behaviour, or outright hard to locate runtime errors.
Coming to the crux of the issue, I rewrote get_default_argparser
to this:
from copy import deepcopy
import argparse as ap
DEFAULT_FLAGS = {
"--version": {
"help": "Specify a version to install. Default is latest.",
"required": False,
"default": None
},
"--keep": {
"help": "Specify if downloaded files will be kept after finishing.",
"required": False,
"default": False,
"action": "store_true"
},
"--temporary": {
"help": "Specify temporary directory files will be downloaded at. Default is /tmp/",
"required": False,
"default": None,
"type": str
},
"--destination": {
"help": "Specify installation directory.",
"required": False,
"default": None,
"type": str
},
"--unsafe": {
"help": "Specify if file verification will be skipped. Set by default if unsupported by the repository.",
"required": False,
"default": False,
"action": "store_true"
},
}
def get_default_argparser(description, version=True, keep=True, temporary=True, destination=True, unsafe=True):
flags_dict = deepcopy(DEFAULT_FLAGS)
if not version:
del flags_dict["--version"]
if not keep:
del flags_dict["--keep"]
if not temporary:
del flags_dict["--temporary"]
if not destination:
del flags_dict["--destination"]
if not unsafe:
del flags_dict["--unsafe"]
p = ap.ArgumentParser(description=description)
for argname, argopts in flags_dict.items():
p.add_argument(argname, **argopts)
return p
I don't think any amount of documentation is saving this function, and this is not only a code smell, but looks like a ticking time bomb in an expanding code base that needs to be flexible enough to support different requirements. However, I can't think of a good enough way to reimplement this without satisfying the following two requirements:
- Somewhat concise: it requires less boilerplate than adding these arguments to every dependent (manually), and is less error prone
- Self explanatory: it requires little to no explanation in order to understand how it works and what's going on behind the scenes
I thought about making a class that'll have to be passed as an argument to get_default_argparser
, with inverse flags in it's constructor in order to eliminate unneeded behaviour (arguments), however I have a strong feeling this is 2 steps backwards, and it's probably not as self-explanatory as it sounds.
EDIT: J_H's answer is a great one and probably the ideal way for any other person reading this question; however this is not the ideal way to solve my specific issue in my specific usecase, and perhaps it's my fault I didn't disclose it in the first place. The reason I'm looking a design pattern and not to mirate to a modern & better library is twofold: these scripts intentionally run without importing external libraries (#!/usr/bin/env -S python3 -S -OO
shebang) that aren't included with <=python3.12 for somewhat obscure reasons, but one of the benefits of "update-utils" I'm writing is the lack of third party libraries, making it super fast to spin up the python VM if need be; since this is a building block for the remainder of my scripts, I'd like for this option to remain if possible. The second reason J_H's answer doesn't fit my usecase, is because this module and subsequent modules meant to run in the global python ENV, and I would like to avoid contamination, or relying on external libraries. This is a little hypocritical from me, but the only external library all of these scripts depend on is requests
, of which only update-utils
interacts with; scripts should barely know of it's existence other than catching Exception(s).
Like I said, if my usecase wasn't so super-specific, this is probably the "correct" answer; however I'm specifically looking for A Design Pattern to provide a modular, but scalable, "batteries-included" implementation of an instance of a class.
2 Answers 2
I love argparse, I have used it in many projects.
Nowadays I usually use
typer
instead. It's just too easy.
Mention some type annotations,
and the --help
comes "for free".
a good enough way to re-implement this ... [concise, self explanatory]
Yes, I agree with you.
Defining a new TempDir
class
with the business logic you need
is the appropriate way to model this.
Throw in a suitable type annotation
def main(output_folder: TempDir = TempDir('/tmp')) -> None:
...
if __name__ == '__main__':
typer.run(main)
and you're halfway toward making your users happy.
-
2+1 I was just going to comment on the question: Save yourself the trouble. Skip argparse and use typer.JimmyJames– JimmyJames11/15/2023 19:40:54Commented Nov 15, 2023 at 19:40
-
This is a good answer, however it doesn't fit my specific usecase; see edit, what I wanted to clarify was too much to write as a comment. As a sidenote, perhaps since it's late here, I don't understand the pattern you're describing; should I model the arguments of
get_default_argparser
with classes each one, or should I have a "central class" that describes the intended behaviour (as noted in my question)?g_elef– g_elef11/15/2023 20:05:25Commented Nov 15, 2023 at 20:05
I decided to go for a Builder pattern; like amon mentioned in the comments of the question, this was the wrong abstraction in the first place.
Here's the end result:
class ArgumentParserBuilder:
DEFAULT_VERSION: tuple[(str, ...), dict[...]] = (
("-v", "--version"),
{
"help": "Specify a version to install.",
"required": False,
"default": None
}
)
DEFAULT_KEEP: tuple[(str, ...), dict[...]] = (
("-k", "--keep"),
{
"help": "Specify if temporary file cleanup will be performed.",
"required": False,
"default": False,
"action": "store_true"
}
)
DEFAULT_TEMPORARY: tuple[(str, ...), dict[...]] = (
("-t", "--temporary"),
{
"help": "Specify temporary (download) directory files.",
"required": False,
"default": None,
"type": str
}
)
DEFAULT_DESTINATION: tuple[(str, ...), dict[...]] = (
("-d", "--destination"),
{
"help": "Specify installation directory.",
"required": False,
"default": None,
"type": str
}
)
DEFAULT_UNSAFE: tuple[(str, ...), dict[...]] = (
("-u", "--unsafe"),
{
"help": "Specify if file verification will be skipped.",
"required": False,
"default": False,
"action": "store_true"
}
)
def __init__(self, description: str):
self.parser = ap.ArgumentParser(description=description)
def add_version(self) -> Self:
flags, kwargs = ArgumentParserBuilder.DEFAULT_VERSION
self.parser.add_argument(*flags, **kwargs)
return self
def add_keep(self) -> Self:
flags, kwargs = ArgumentParserBuilder.DEFAULT_KEEP
self.parser.add_argument(*flags, **kwargs)
return self
def add_temporary(self) -> Self:
flags, kwargs = ArgumentParserBuilder.DEFAULT_TEMPORARY
self.parser.add_argument(*flags, **kwargs)
return self
def add_destination(self) -> Self:
flags, kwargs = ArgumentParserBuilder.DEFAULT_DESTINATION
self.parser.add_argument(*flags, **kwargs)
return self
def add_unsafe(self) -> Self:
flags, kwargs = ArgumentParserBuilder.DEFAULT_UNSAFE
self.parser.add_argument(*flags, **kwargs)
return self
def add_arguments(self, flags_kwargs_dict: dict[(str, ...), dict[...]]) -> Self:
for flags, argopts in flags_kwargs_dict.items():
self.parser.add_argument(*flags, **argopts)
return self
def add_mutually_exclusive_group(self, flags_kwargs_dict: dict[(str, ...), dict[...]], required=True) -> Self:
meg = self.parser.add_mutually_exclusive_group(required=required)
for flags, argopts in flags_kwargs_dict.items():
meg.add_argument(*flags, **argopts)
return self
def build(self) -> ap.ArgumentParser:
return self.parser
The class might look a little like a god class at first glance, however it's intent is very specific and results in pretty robust code as far as I can tell.
Explore related questions
See similar questions with these tags.
add_version_flag(p)
. That said, yourget_default_argparser(**kwargs)
function could also be made simpler by giving it a set of names to skip, e.g.without={"--destination"}