Apply suggestions from code review

pypa · Jul 24, 2024 · 6a3061d · 6a3061d
1 parent 5b77887
commit 6a3061d
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 11 deletions.
diff --git a/src/pipx/backend.py b/src/pipx/backend.py
@@ -8,14 +8,14 @@
 
 
 def path_to_exec(executable: str, installer: bool = False) -> str:
-    if executable == "venv" or "pip":
+    if executable in ("venv", "pip"):
         return executable
     path = shutil.which(executable)
     if path:
         return path
     elif installer:
-        logger.warning(f"{executable} not found on PATH. Falling back to pip.")
+        logger.warning(f"'{executable}' not found on PATH. Falling back to 'pip'.")
         return ""
     else:
-        logger.warning(f"{executable} not found on PATH. Falling back to venv.")
+        logger.warning(f"'{executable}' not found on PATH. Falling back to 'venv'.")
         return ""
diff --git a/src/pipx/venv.py b/src/pipx/venv.py
@@ -112,7 +112,7 @@ def __init__(
 
         if not path_to_exec(self.backend):
             self.backend = "venv"
-        if not path_to_exec(self.installer):
+        if not path_to_exec(self.installer, installer=True):
             self.installer = "pip"
 
     def check_upgrade_shared_libs(self, verbose: bool, pip_args: List[str], force_upgrade: bool = False):
@@ -290,7 +290,7 @@ def install_package(
             # no logging because any errors will be specially logged by
             #   subprocess_post_check_handle_pip_error()
             install_process = self._run_installer(
-                self.installer, [*pip_args, package_or_url], quiet=True, log_stdout=False, log_stderr=False
+                [*pip_args, package_or_url], quiet=True, log_stdout=False, log_stderr=False
             )
         subprocess_post_check_handle_pip_error(install_process)
         if install_process.returncode:
@@ -329,7 +329,7 @@ def install_unmanaged_packages(self, requirements: List[str], pip_args: List[str
             # no logging because any errors will be specially logged by
             #   subprocess_post_check_handle_pip_error()
             install_process = self._run_installer(
-                self.installer, [*pip_args, *requirements], quiet=True, log_stdout=False, log_stderr=False
+                [*pip_args, *requirements], quiet=True, log_stdout=False, log_stderr=False
             )
         subprocess_post_check_handle_pip_error(install_process)
         if install_process.returncode:
@@ -339,7 +339,7 @@ def install_package_no_deps(self, package_or_url: str, pip_args: List[str]) -> s
         with animate(f"determining package name from {package_or_url!r}", self.do_animation):
             old_package_set = self.list_installed_packages()
             # TODO: rename pip_args to installer_args? But we can keep pip_args as implicit one
-            install_process = self._run_installer(self.installer, [*pip_args, package_or_url], no_deps=True)
+            install_process = self._run_installer([*pip_args, package_or_url], no_deps=True)
 
         subprocess_post_check(install_process, raise_error=False)
         if install_process.returncode:
@@ -464,7 +464,7 @@ def has_package(self, package_name: str) -> bool:
     def upgrade_package_no_metadata(self, package_name: str, pip_args: List[str]) -> None:
         logger.info("Upgrading %s", package_descr := full_package_description(package_name, package_name))
         with animate(f"upgrading {package_descr}", self.do_animation):
-            upgrade_process = self._run_installer(self.installer, [*pip_args, "--upgrade", package_name])
+            upgrade_process = self._run_installer([*pip_args, "--upgrade", package_name])
         subprocess_post_check(upgrade_process)
 
     def upgrade_package(
@@ -479,7 +479,7 @@ def upgrade_package(
     ) -> None:
         logger.info("Upgrading %s", package_descr := full_package_description(package_name, package_or_url))
         with animate(f"upgrading {package_descr}", self.do_animation):
-            upgrade_process = self._run_installer(self.installer, [*pip_args, "--upgrade", package_or_url])
+            upgrade_process = self._run_installer([*pip_args, "--upgrade", package_or_url])
         subprocess_post_check(upgrade_process)
 
         self.update_package_metadata(
@@ -494,7 +494,6 @@ def upgrade_package(
 
     def _run_installer(
         self,
-        installer: str,
         cmd: List[str],
         quiet: bool = True,
         no_deps: bool = False,
@@ -503,7 +502,7 @@ def _run_installer(
     ) -> "CompletedProcess[str]":
         # do not use -q with `pip install` so subprocess_post_check_pip_errors
         #   has more information to analyze in case of failure.
-        if installer != "uv":
+        if self.installer != "uv":
             return self._run_pip(
                 ["--no-input", "install"] + (["--no-dependencies"] if no_deps else []) + cmd, quiet=quiet
             )