From 3a5b4df60c0787490933278c789a3605a0525203 Mon Sep 17 00:00:00 2001 From: M Bussonnier Date: Mon, 26 Aug 2024 13:43:16 +0200 Subject: [PATCH] Do not use f-strings in log calls. It is in general bad practice; in particular if one want a structured logging handler that store the template string and the values separately, using f-strings makes it not possible. I belive there is also a question of actually calling the formatter on the interpolated values which is not done if the loging level is low enough. WHile it might not be too critical for micropip, I belive it is good practive to show the example. --- micropip/_commands/uninstall.py | 18 ++++++++++-------- micropip/transaction.py | 8 ++++---- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/micropip/_commands/uninstall.py b/micropip/_commands/uninstall.py index a22f3e7..de19899 100644 --- a/micropip/_commands/uninstall.py +++ b/micropip/_commands/uninstall.py @@ -39,7 +39,7 @@ def uninstall(packages: str | list[str], *, verbose: bool | int = False) -> None dist = importlib.metadata.distribution(package) distributions.append(dist) except importlib.metadata.PackageNotFoundError: - logger.warning(f"Skipping '{package}' as it is not installed.") + logger.warning("Skipping 's' as it is not installed.", package) for dist in distributions: # Note: this value needs to be retrieved before removing files, as @@ -47,7 +47,7 @@ def uninstall(packages: str | list[str], *, verbose: bool | int = False) -> None name = dist.name version = dist.version - logger.info(f"Found existing installation: {name} {version}") + logger.info("Found existing installation: %s %s", name, version) root = get_root(dist) files = get_files_in_distribution(dist) @@ -64,7 +64,9 @@ def uninstall(packages: str | list[str], *, verbose: bool | int = False) -> None continue logger.warning( - f"A file '{file}' listed in the metadata of '{name}' does not exist.", + "A file '%s' listed in the metadata of '%s' does not exist.", + file, + name, ) continue @@ -80,18 +82,18 @@ def uninstall(packages: str | list[str], *, verbose: bool | int = False) -> None directory.rmdir() except OSError: logger.warning( - f"A directory '{directory}' is not empty after uninstallation of '{name}'. " + "A directory '%s' is not empty after uninstallation of '%s'. " "This might cause problems when installing a new version of the package. ", + directory, + name, ) if hasattr(loadedPackages, name): delattr(loadedPackages, name) else: # This should not happen, but just in case - logger.warning( - f"a package '{name}' was not found in loadedPackages.", - ) + logger.warning("a package '%s' was not found in loadedPackages.", name) - logger.info(f"Successfully uninstalled {name}-{version}") + logger.info("Successfully uninstalled %s-%s", name, version) importlib.invalidate_caches() diff --git a/micropip/transaction.py b/micropip/transaction.py index bbc91e2..93f51ea 100644 --- a/micropip/transaction.py +++ b/micropip/transaction.py @@ -140,7 +140,7 @@ def eval_marker(e: dict[str, str]) -> bool: satisfied, ver = self.check_version_satisfied(req) if satisfied: - logger.info(f"Requirement already satisfied: {req} ({ver})") + logger.info("Requirement already satisfied: %s (%s)", req, ver) return try: @@ -193,7 +193,7 @@ async def _add_requirement_from_package_index(self, req: Requirement): # installed the wheel? satisfied, ver = self.check_version_satisfied(req) if satisfied: - logger.info(f"Requirement already satisfied: {req} ({ver})") + logger.info("Requirement already satisfied: %s (%s)", req, ver) await self.add_wheel(wheel, req.extras, specifier=str(req.specifier)) @@ -228,8 +228,8 @@ async def add_wheel( version=str(wheel.version), ) - logger.info(f"Collecting {wheel.name}{specifier}") - logger.info(f" Downloading {wheel.url.split('/')[-1]}") + logger.info("Collecting %s%s", wheel.name, specifier) + logger.info(" Downloading %s", wheel.url.split("/")[-1]) await wheel.download(self.fetch_kwargs) if self.deps: