From c6c51e5897c32cfda5e7fe7fdeeec2be69010d62 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sat, 24 Aug 2024 18:54:34 +0300 Subject: [PATCH 1/2] Apply repo-review suggestion RF003 RF003: src directory doesn't need to be specified anymore (0.6+) Ruff now (0.6+) looks in the src directory by default. The src setting doesn't need to be specified if it's just set to `["src"]`. --- pyproject.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 443de11e7b..3cd3bfa0af 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -62,9 +62,6 @@ version.source = "vcs" [tool.ruff] line-length = 121 -src = [ - "src", -] lint.extend-select = [ "A", "B", From 40c7227b1a95a28eef7ed1990a1ebad06d8dbd5e Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sat, 24 Aug 2024 18:57:23 +0300 Subject: [PATCH 2/2] Apply repo-review MyPy suggestions MY103: MyPy warn unreachable Must have `warn_unreachable` (true/false) to pass this check. There are occasionally false positives (often due to platform or Python version static checks), so it's okay to set it to false if you need to. But try it first - it can catch real bugs too. MY104: MyPy enables ignore-without-code Must have `"ignore-without-code"` in `enable_error_code = [...]`. This will force all skips in your project to include the error code, which makes them more readable, and avoids skipping something unintended. MY105: MyPy enables redundant-expr Must have `"redundant-expr"` in `enable_error_code = [...]`. This helps catch useless lines of code, like checking the same condition twice. MY106: MyPy enables truthy-bool Must have `"truthy-bool"` in `enable_error_code = []`. This catches mistakes in using a value as truthy if it cannot be falsy. --- pyproject.toml | 4 ++++ src/pipx/animate.py | 2 +- src/pipx/commands/environment.py | 2 +- src/pipx/commands/inject.py | 2 +- src/pipx/commands/run.py | 4 ++-- src/pipx/commands/uninject.py | 2 +- src/pipx/main.py | 4 ++-- src/pipx/paths.py | 14 +++++++------- src/pipx/shared_libs.py | 2 +- tests/test_upgrade_shared.py | 4 ++-- 10 files changed, 22 insertions(+), 18 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3cd3bfa0af..b94068e06f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -119,6 +119,10 @@ title_format = "## [{version}](https://github.com/pypa/pipx/tree/{version}) - {p issue_format = "[#{issue}](https://github.com/pypa/pipx/issues/{issue})" package = "pipx" +[tool.mypy] +warn_unreachable = true +enable_error_code = [ "ignore-without-code", "redundant-expr", "truthy-bool" ] + [[tool.mypy.overrides]] module = [ "pycowsay.*", diff --git a/src/pipx/animate.py b/src/pipx/animate.py index ed1dcd6f30..ec705ec68a 100644 --- a/src/pipx/animate.py +++ b/src/pipx/animate.py @@ -101,7 +101,7 @@ def print_animation( def win_cursor(visible: bool) -> None: if sys.platform != "win32": # hello mypy return - ci = _CursorInfo() + ci = _CursorInfo() # type: ignore[unreachable] handle = ctypes.windll.kernel32.GetStdHandle(-11) ctypes.windll.kernel32.GetConsoleCursorInfo(handle, ctypes.byref(ci)) ci.visible = visible diff --git a/src/pipx/commands/environment.py b/src/pipx/commands/environment.py index e82408104b..963e4dc209 100644 --- a/src/pipx/commands/environment.py +++ b/src/pipx/commands/environment.py @@ -38,7 +38,7 @@ def environment(value: str) -> ExitCode: "PIPX_HOME_ALLOW_SPACE": str(paths.ctx.allow_spaces_in_home_path).lower(), } if value is None: - print("Environment variables (set by user):") + print("Environment variables (set by user):") # type: ignore[unreachable] print("") for env_variable in ENVIRONMENT_VARIABLES: env_value = os.getenv(env_variable, "") diff --git a/src/pipx/commands/inject.py b/src/pipx/commands/inject.py index b315b59c57..f579576b38 100644 --- a/src/pipx/commands/inject.py +++ b/src/pipx/commands/inject.py @@ -32,7 +32,7 @@ def inject_dep( ) -> bool: logger.debug("Injecting package %s", package_spec) - if not venv_dir.exists() or not next(venv_dir.iterdir()): + if not venv_dir.exists() or next(venv_dir.iterdir(), None) is None: raise PipxError( f""" Can't inject {package_spec!r} into nonexistent Virtual Environment diff --git a/src/pipx/commands/run.py b/src/pipx/commands/run.py index c68dfcf8a2..5cd9683046 100644 --- a/src/pipx/commands/run.py +++ b/src/pipx/commands/run.py @@ -198,11 +198,11 @@ def run( # we can't parse this as a package package_name = app - content = None if spec is not None else maybe_script_content(app, is_path) + content = None if spec is not None else maybe_script_content(app, is_path) # type: ignore[redundant-expr] if content is not None: run_script(content, app_args, python, pip_args, venv_args, verbose, use_cache) else: - package_or_url = spec if spec is not None else app + package_or_url = spec if spec is not None else app # type: ignore[redundant-expr] run_package( package_name, package_or_url, diff --git a/src/pipx/commands/uninject.py b/src/pipx/commands/uninject.py index 9711275a2f..9b36132a71 100644 --- a/src/pipx/commands/uninject.py +++ b/src/pipx/commands/uninject.py @@ -125,7 +125,7 @@ def uninject( ) -> ExitCode: """Returns pipx exit code""" - if not venv_dir.exists() or not next(venv_dir.iterdir()): + if not venv_dir.exists() or next(venv_dir.iterdir(), None) is None: raise PipxError(f"Virtual environment {venv_dir.name} does not exist.") venv = Venv(venv_dir, verbose=verbose) diff --git a/src/pipx/main.py b/src/pipx/main.py index 4800f64313..34aa6bb486 100644 --- a/src/pipx/main.py +++ b/src/pipx/main.py @@ -277,7 +277,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar not args.no_cache, ) # We should never reach here because run() is NoReturn. - return ExitCode(1) + return ExitCode(1) # type: ignore[unreachable] elif args.command == "install": return commands.install( None, @@ -409,7 +409,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar python_flag_passed=python_flag_passed, ) elif args.command == "runpip": - if not venv_dir: + if not venv_dir: # type: ignore[truthy-bool] raise PipxError("Developer error: venv_dir is not defined.") return commands.run_pip(package, venv_dir, args.pipargs, args.verbose) elif args.command == "ensurepath": diff --git a/src/pipx/paths.py b/src/pipx/paths.py index 0b676b641f..5970dd5368 100644 --- a/src/pipx/paths.py +++ b/src/pipx/paths.py @@ -107,13 +107,13 @@ def shared_libs(self) -> Path: return (self._base_shared_libs or self.home / "shared").resolve() def make_local(self) -> None: - self._base_home = OVERRIDE_PIPX_HOME or get_expanded_environ("PIPX_HOME") + self._base_home = OVERRIDE_PIPX_HOME or get_expanded_environ("PIPX_HOME") # type: ignore[redundant-expr] self._default_home = DEFAULT_PIPX_HOME - self._base_bin = OVERRIDE_PIPX_BIN_DIR or get_expanded_environ("PIPX_BIN_DIR") + self._base_bin = OVERRIDE_PIPX_BIN_DIR or get_expanded_environ("PIPX_BIN_DIR") # type: ignore[redundant-expr] self._default_bin = DEFAULT_PIPX_BIN_DIR - self._base_man = OVERRIDE_PIPX_MAN_DIR or get_expanded_environ("PIPX_MAN_DIR") + self._base_man = OVERRIDE_PIPX_MAN_DIR or get_expanded_environ("PIPX_MAN_DIR") # type: ignore[redundant-expr] self._default_man = DEFAULT_PIPX_MAN_DIR - self._base_shared_libs = OVERRIDE_PIPX_SHARED_LIBS or get_expanded_environ("PIPX_SHARED_LIBS") + self._base_shared_libs = OVERRIDE_PIPX_SHARED_LIBS or get_expanded_environ("PIPX_SHARED_LIBS") # type: ignore[redundant-expr] self._default_log = Path(user_log_path("pipx")) self._default_cache = Path(user_cache_path("pipx")) self._default_trash = self._default_home / "trash" @@ -121,11 +121,11 @@ def make_local(self) -> None: self._home_exists = self._base_home is not None or any(fallback.exists() for fallback in FALLBACK_PIPX_HOMES) def make_global(self) -> None: - self._base_home = OVERRIDE_PIPX_GLOBAL_HOME or get_expanded_environ("PIPX_GLOBAL_HOME") + self._base_home = OVERRIDE_PIPX_GLOBAL_HOME or get_expanded_environ("PIPX_GLOBAL_HOME") # type: ignore[redundant-expr] self._default_home = DEFAULT_PIPX_GLOBAL_HOME - self._base_bin = OVERRIDE_PIPX_GLOBAL_BIN_DIR or get_expanded_environ("PIPX_GLOBAL_BIN_DIR") + self._base_bin = OVERRIDE_PIPX_GLOBAL_BIN_DIR or get_expanded_environ("PIPX_GLOBAL_BIN_DIR") # type: ignore[redundant-expr] self._default_bin = DEFAULT_PIPX_GLOBAL_BIN_DIR - self._base_man = OVERRIDE_PIPX_GLOBAL_MAN_DIR or get_expanded_environ("PIPX_GLOBAL_MAN_DIR") + self._base_man = OVERRIDE_PIPX_GLOBAL_MAN_DIR or get_expanded_environ("PIPX_GLOBAL_MAN_DIR") # type: ignore[redundant-expr] self._default_man = DEFAULT_PIPX_GLOBAL_MAN_DIR self._default_log = self._default_home / "logs" self._default_cache = self._default_home / ".cache" diff --git a/src/pipx/shared_libs.py b/src/pipx/shared_libs.py index 2020c8923c..97c79bbe1e 100644 --- a/src/pipx/shared_libs.py +++ b/src/pipx/shared_libs.py @@ -114,7 +114,7 @@ def upgrade(self, *, pip_args: List[str], verbose: bool = False, raises: bool = return if pip_args is None: - pip_args = [] + pip_args = [] # type: ignore[unreachable] logger.info(f"Upgrading shared libraries in {self.root}") diff --git a/tests/test_upgrade_shared.py b/tests/test_upgrade_shared.py index 6e9179fd8f..fda786be84 100644 --- a/tests/test_upgrade_shared.py +++ b/tests/test_upgrade_shared.py @@ -23,7 +23,7 @@ def test_upgrade_shared(shared_libs, capsys, caplog): assert "Upgrading shared libraries in" in caplog.text assert "Already upgraded libraries in" not in caplog.text assert shared_libs.has_been_updated_this_run is True - assert shared_libs.is_valid is True + assert shared_libs.is_valid is True # type: ignore[unreachable] shared_libs.has_been_updated_this_run = False assert run_pipx_cli(["upgrade-shared", "-v"]) == 0 captured = capsys.readouterr() @@ -59,7 +59,7 @@ def pip_version(): assert shared_libs.is_valid is False assert run_pipx_cli(["upgrade-shared", "-v", "--pip-args=pip==24.0"]) == 0 assert shared_libs.is_valid is True - assert pip_version() == "24.0" + assert pip_version() == "24.0" # type: ignore[unreachable] shared_libs.has_been_updated_this_run = False # reset for next run assert run_pipx_cli(["upgrade-shared", "-v", "--pip-args=pip==23.3.2"]) == 0 assert shared_libs.is_valid is True