Skip to content

Commit

Permalink
Do not use f-strings in log calls.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Carreau committed Aug 26, 2024
1 parent 8807c59 commit 3a5b4df
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 12 deletions.
18 changes: 10 additions & 8 deletions micropip/_commands/uninstall.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ 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
# dist.name uses metadata file to get the name
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)
Expand All @@ -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
Expand All @@ -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()
8 changes: 4 additions & 4 deletions micropip/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 3a5b4df

Please sign in to comment.