ronantakizawa/codereview-bench huggingface dataset:
Repository: pypa/pip
File: tests/unit/test_packaging.py (Python)

Code before:
        ((3, 6, 5), None, True),
    ],
)
def test_check_requires_python(
    version_info: Tuple[int, int, int], requires_python: Optional[str], expected: bool
) -> None:
    actual = check_requires_python(requires_python, version_info)
    assert actual == expected


def test_check_requires_python__invalid() -> None:
    """
    Test an invalid Requires-Python value.
    """
    with pytest.raises(specifiers.InvalidSpecifier):
        check_requires_python("invalid", (3, 6, 5))


def test_get_or_create_caching() -> None:
    """test caching of get_or_create requirement"""
    teststr = "affinegap==1.10"
    from_helper = get_requirement(teststr)
    freshly_made = Requirement(teststr)
    # Requirement doesn't have an equality operator (yet) so test
    # equality of attribute for list of attributes
    for iattr in ["name", "url", "extras", "specifier", "marker"]:
        assert getattr(from_helper, iattr) == getattr(freshly_made, iattr)
    assert not (get_requirement(teststr) is Requirement(teststr))
    assert get_requirement(teststr) is get_requirement(teststr)


Review comment (suggestion):
```suggestion

    # Requirement doesn't have an equality operator (yet) so test
    # equality of attribute for list of attributes
```

nit: blank line, to visually separate the assertions from the "setup" for the test.

Code after:
        ((3, 6, 5), None, True),
    ],
)
def test_check_requires_python(
    version_info: Tuple[int, int, int], requires_python: Optional[str], expected: bool
) -> None:
    actual = check_requires_python(requires_python, version_info)
    assert actual == expected


def test_check_requires_python__invalid() -> None:
    """
    Test an invalid Requires-Python value.
    """
    with pytest.raises(specifiers.InvalidSpecifier):
        check_requires_python("invalid", (3, 6, 5))


def test_get_or_create_caching() -> None:
    """test caching of get_or_create requirement"""
    teststr = "affinegap==1.10"
    from_helper = get_requirement(teststr)
    freshly_made = Requirement(teststr)

    # Requirement doesn't have an equality operator (yet) so test
    # equality of attribute for list of attributes
    for iattr in ["name", "url", "extras", "specifier", "marker"]:
        assert getattr(from_helper, iattr) == getattr(freshly_made, iattr)
    assert get_requirement(teststr) is not Requirement(teststr)
    assert get_requirement(teststr) is get_requirement(teststr)
Repository: pypa/pip
File: src/pip/_internal/commands/check.py (Python)

Code before:

        for project_name in missing:
            version = package_set[project_name].version
            for dependency in missing[project_name]:
                write_output(
                    "%s %s requires %s, which is not installed.",
                    project_name,
                    version,
                    dependency[0],
                )

        for project_name in conflicting:
            version = package_set[project_name].version
            for dep_name, dep_version, req in conflicting[project_name]:
                write_output(
                    "%s %s has requirement %s, but you have %s %s.",
                    project_name,
                    version,
                    req,
                    dep_name,
                    dep_version,
                )
        for package in unsupported:
            write_output(
                "%s %s is not supported on this platform",
                package.canonical_name,
                package.version,
            )
        if missing or conflicting or unsupported or parsing_probs:
            return ERROR
        else:
            write_output("No broken requirements found.")
            return SUCCESS


Review comment (suggestion):
For user-facing messages `raw_name` is generally preferred.

Code after:
                get_default_environment().iter_installed_distributions(),
                get_supported(),
            )
        )

        for project_name in missing:
            version = package_set[project_name].version
            for dependency in missing[project_name]:
                write_output(
                    "%s %s requires %s, which is not installed.",
                    project_name,
                    version,
                    dependency[0],
                )

        for project_name in conflicting:
            version = package_set[project_name].version
            for dep_name, dep_version, req in conflicting[project_name]:
                write_output(
                    "%s %s has requirement %s, but you have %s %s.",
                    project_name,
                    version,
                    req,
                    dep_name,
                    dep_version,
                )
        for package in unsupported:
            write_output(
                "%s %s is not supported on this platform",
                package.raw_name,
                package.version,
            )
        if missing or conflicting or parsing_probs or unsupported:
            return ERROR
        else:
            write_output("No broken requirements found.")
            return SUCCESS
Repository: pypa/pip
File: src/pip/_internal/cli/index_command.py (Python)

Code before:
--uptodate) don't need waste time importing PipSession and friends.
"""

import logging
import os
import sys
from optparse import Values
from typing import TYPE_CHECKING, List, Optional

from pip._vendor import certifi

from pip._internal.cli.base_command import Command
from pip._internal.cli.command_context import CommandContextMixIn

if TYPE_CHECKING:
    from ssl import SSLContext

    from pip._internal.network.session import PipSession

logger = logging.getLogger(__name__)


def _create_truststore_ssl_context() -> Optional["SSLContext"]:
    if sys.version_info < (3, 10):
        logger.warning("Disabling truststore because Python version isn't 3.10+")
        return None

    try:
        import ssl
    except ImportError:
        logger.warning("Disabling truststore since ssl support is missing")
        return None

    try:
        from pip._vendor import truststore
    except ImportError:
        logger.warning("Disabling truststore because platform isn't supported")
        return None

    ctx = truststore.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
    ctx.load_verify_locations(certifi.where())
    return ctx


class SessionCommandMixin(CommandContextMixIn):
    """
    A class mixin for command classes needing _build_session().
    """

    def __init__(self) -> None:
        super().__init__()

Review comment (bug):
![image](https://github.com/pypa/pip/assets/63936253/107c1fb1-c363-4493-9ad1-79cf20c990b2)

To be honest, emitting a warning on every single invocation of pip on Python 3.9 and lower is plain annoying. This can be a debug or verbose log.

Code after:
--uptodate) don't need waste time importing PipSession and friends.
"""

import logging
import os
import sys
from optparse import Values
from typing import TYPE_CHECKING, List, Optional

from pip._vendor import certifi

from pip._internal.cli.base_command import Command
from pip._internal.cli.command_context import CommandContextMixIn

if TYPE_CHECKING:
    from ssl import SSLContext

    from pip._internal.network.session import PipSession

logger = logging.getLogger(__name__)


def _create_truststore_ssl_context() -> Optional["SSLContext"]:
    if sys.version_info < (3, 10):
        logger.debug("Disabling truststore because Python version isn't 3.10+")
        return None

    try:
        import ssl
    except ImportError:
        logger.warning("Disabling truststore since ssl support is missing")
        return None

    try:
        from pip._vendor import truststore
    except ImportError:
        logger.warning("Disabling truststore because platform isn't supported")
        return None

    ctx = truststore.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
    ctx.load_verify_locations(certifi.where())
    return ctx


class SessionCommandMixin(CommandContextMixIn):
    """
    A class mixin for command classes needing _build_session().
    """

    def __init__(self) -> None:
        super().__init__()
Repository: pypa/pip
File: src/pip/_internal/cli/cmdoptions.py (Python)

Code before:
    default=False,
    help=(
        "Run pip in an isolated mode, ignoring environment variables and user "
        "configuration."
    ),
)

require_virtualenv: Callable[..., Option] = partial(
    Option,
    "--require-virtualenv",
    "--require-venv",
    dest="require_venv",
    action="store_true",
    default=False,
    help=(
        "Allow pip to only run in a virtual environment; "
        "exit with an error otherwise."
    ),
)

override_externally_managed: Callable[..., Option] = partial(
    Option,
    "--break-system-packages",
    dest="override_externally_managed",
    action="store_true",
    help="Allow pip to install into an EXTERNALLY-MANAGED Python installation",
)

python: Callable[..., Option] = partial(
    Option,
    "--python",
    dest="python",
    help="Run pip with the specified Python interpreter.",
)

verbose: Callable[..., Option] = partial(
    Option,
    "-v",
    "--verbose",
    dest="verbose",
    action="count",
    default=0,
    help="Give more output. Option is additive, and can be used up to 3 times.",
)

no_color: Callable[..., Option] = partial(
    Option,
    "--no-color",
    dest="no_color",
    action="store_true",
    default=False,

Review comment (suggestion):
```suggestion
    help="Allow pip to modify an EXTERNALLY-MANAGED Python installation",
```

Code after:
    default=False,
    help=(
        "Run pip in an isolated mode, ignoring environment variables and user "
        "configuration."
    ),
)

require_virtualenv: Callable[..., Option] = partial(
    Option,
    "--require-virtualenv",
    "--require-venv",
    dest="require_venv",
    action="store_true",
    default=False,
    help=(
        "Allow pip to only run in a virtual environment; "
        "exit with an error otherwise."
    ),
)

override_externally_managed: Callable[..., Option] = partial(
    Option,
    "--break-system-packages",
    dest="override_externally_managed",
    action="store_true",
    help="Allow pip to modify an EXTERNALLY-MANAGED Python installation",
)

python: Callable[..., Option] = partial(
    Option,
    "--python",
    dest="python",
    help="Run pip with the specified Python interpreter.",
)

verbose: Callable[..., Option] = partial(
    Option,
    "-v",
    "--verbose",
    dest="verbose",
    action="count",
    default=0,
    help="Give more output. Option is additive, and can be used up to 3 times.",
)

no_color: Callable[..., Option] = partial(
    Option,
    "--no-color",
    dest="no_color",
    action="store_true",
    default=False,
Repository: pypa/pip
File: src/pip/_internal/cli/cmdoptions.py (Python)

Code before:
    type="package_name",
    help="Exclude specified package from the output",
)


no_python_version_warning: Callable[..., Option] = partial(
    Option,
    "--no-python-version-warning",
    dest="no_python_version_warning",
    action="store_true",
    default=False,
    help="Silence deprecation warnings for upcoming unsupported Pythons.",
)


use_new_feature: Callable[..., Option] = partial(
    Option,
    "--use-feature",
    dest="features_enabled",
    metavar="feature",
    action="append",
    default=[],
    choices=[
        "fast-deps",
        "truststore",
        "no-binary-enable-wheel-cache",  # now always on
    ],
    help="Enable new functionality, that may be backward incompatible.",
)

use_deprecated_feature: Callable[..., Option] = partial(
    Option,
    "--use-deprecated",
    dest="deprecated_features_enabled",
    metavar="feature",
    action="append",
    default=[],
    choices=[
        "legacy-resolver",
    ],
    help=("Enable deprecated functionality, that will be removed in the future."),
)


##########
# groups #
##########

general_group: Dict[str, Any] = {
    "name": "General Options",
    "options": [

Review comment (refactor):
Why not remove this, as we've done with other `--use-feature` flags?

Code after:
    type="package_name",
    help="Exclude specified package from the output",
)


no_python_version_warning: Callable[..., Option] = partial(
    Option,
    "--no-python-version-warning",
    dest="no_python_version_warning",
    action="store_true",
    default=False,
    help="Silence deprecation warnings for upcoming unsupported Pythons.",
)


# Features that are now always on. A warning is printed if they are used.
ALWAYS_ENABLED_FEATURES = [
    "no-binary-enable-wheel-cache",  # always on since 23.1
]

use_new_feature: Callable[..., Option] = partial(
    Option,
    "--use-feature",
    dest="features_enabled",
    metavar="feature",
    action="append",
    default=[],
    choices=[
        "fast-deps",
        "truststore",
    ]
    + ALWAYS_ENABLED_FEATURES,
    help="Enable new functionality, that may be backward incompatible.",
)

use_deprecated_feature: Callable[..., Option] = partial(
    Option,
    "--use-deprecated",
    dest="deprecated_features_enabled",
    metavar="feature",
    action="append",
    default=[],
    choices=[
        "legacy-resolver",
    ],
    help=("Enable deprecated functionality, that will be removed in the future."),
)


##########
# groups #
Repository: pypa/pip
File: tests/data/packages/BrokenEmitsUTF8/setup.py (Python)

Code before:
import sys
from distutils.core import setup


class FakeError(Exception):
    pass


if sys.argv[1] in ("install", "bdist_wheel"):
    if hasattr(sys.stdout, "buffer"):
        sys.stdout.buffer.write(b"\nThis package prints out UTF-8 stuff like:\n")
        sys.stdout.buffer.write("* return type of ‘main’ is not ‘int’\n".encode())
        sys.stdout.buffer.write(
            "* Björk Guðmundsdóttir [ˈpjœr̥k ˈkvʏðmʏntsˌtoʊhtɪr]".encode()
        )
    else:
        pass
        sys.stdout.write("\nThis package prints out UTF-8 stuff like:\n")
        sys.stdout.write(
            "* return type of \xe2\x80\x98main\xe2\x80\x99 is not \xe2\x80\x98int\xe2\x80\x99\n"
        )
        sys.stdout.write(
            "* Bj\xc3\xb6rk Gu\xc3\xb0mundsd\xc3\xb3ttir [\xcb\x88pj\xc5\x93r\xcc\xa5k \xcb\x88kv\xca\x8f\xc3\xb0m\xca\x8fnts\xcb\x8cto\xca\x8aht\xc9\xaar]\n"
        )

    raise FakeError("this package designed to fail on install")

setup(
    name="broken",
    version="0.2",
    py_modules=["broken"],
)


Review comment (suggestion):
```suggestion
        sys.stdout.buffer.write("\nThis package prints out UTF-8 stuff like:\n".encode())
```

I prefer this for consistency, so if someone wants to click "apply" that's fine. But I don't care that much, so ignore it if you prefer.

Code after:
import sys
from distutils.core import setup


class FakeError(Exception):
    pass


if sys.argv[1] in ("install", "bdist_wheel"):
    if hasattr(sys.stdout, "buffer"):
        sys.stdout.buffer.write(
            "\nThis package prints out UTF-8 stuff like:\n".encode()
        )
        sys.stdout.buffer.write("* return type of ‘main’ is not ‘int’\n".encode())
        sys.stdout.buffer.write(
            "* Björk Guðmundsdóttir [ˈpjœr̥k ˈkvʏðmʏntsˌtoʊhtɪr]".encode()
        )
    else:
        pass
        sys.stdout.write("\nThis package prints out UTF-8 stuff like:\n")
        sys.stdout.write(
            "* return type of \xe2\x80\x98main\xe2\x80\x99 is not \xe2\x80\x98int\xe2\x80\x99\n"
        )
        sys.stdout.write(
            "* Bj\xc3\xb6rk Gu\xc3\xb0mundsd\xc3\xb3ttir [\xcb\x88pj\xc5\x93r\xcc\xa5k \xcb\x88kv\xca\x8f\xc3\xb0m\xca\x8fnts\xcb\x8cto\xca\x8aht\xc9\xaar]\n"
        )

    raise FakeError("this package designed to fail on install")

setup(
    name="broken",
    version="0.2",
    py_modules=["broken"],
)
Repository: pypa/pip
File: tests/functional/test_install.py (Python)

Code before:
@pytest.fixture
def install_find_links(
    script: PipTestEnvironment,
    data: TestData,
) -> Callable[[Iterable[str], bool, Optional[Path]], TestPipResult]:
    def install(
        args: Iterable[str], dry_run: bool, target_dir: Optional[Path]
    ) -> TestPipResult:
        return script.pip(
            "install",
            *(
                (
                    "--target",
                    str(target_dir),
                )
                if target_dir is not None
                else ()
            ),
            *(("--dry-run",) if dry_run else ()),
            "--no-index",
            "--find-links",
            data.find_links,
            *args,
        )

    return install


@pytest.mark.parametrize(
    "with_target_dir",
    (True, False),
)
def test_install_dry_run_nothing_installed(
    script: PipTestEnvironment,
    tmpdir: Path,
    install_find_links: Callable[[Iterable[str], bool, Optional[Path]], TestPipResult],
    with_target_dir: bool,
) -> None:
    """Test that pip install --dry-run logs what it would install, but doesn't actually
    install anything."""
    if with_target_dir:
        install_dir = tmpdir / "fake-install"
        install_dir.mkdir()
    else:
        install_dir = None

    result = install_find_links(["simple"], True, install_dir)
    assert "Would install simple-3.0" in result.stdout
    assert "Successfully installed" not in result.stdout

    script.assert_not_installed("simple")

Review comment (bug):
Making `install_find_links` into a fixture rather than a simple helper function (that takes script and data as additional arguments) is very confusing to me. I'm a bug fan of keeping things simple. Do we need this extra indirection?

Code after:
def install_find_links(
    script: PipTestEnvironment,
    data: TestData,
    args: Iterable[str],
    *,
    dry_run: bool,
    target_dir: Optional[Path],
) -> TestPipResult:
    return script.pip(
        "install",
        *(
            (
                "--target",
                str(target_dir),
            )
            if target_dir is not None
            else ()
        ),
        *(("--dry-run",) if dry_run else ()),
        "--no-index",
        "--find-links",
        data.find_links,
        *args,
    )


@pytest.mark.parametrize(
    "with_target_dir",
    (True, False),
)
def test_install_dry_run_nothing_installed(
    script: PipTestEnvironment,
    data: TestData,
    tmpdir: Path,
    with_target_dir: bool,
) -> None:
    """Test that pip install --dry-run logs what it would install, but doesn't actually
    install anything."""
    if with_target_dir:
        install_dir = tmpdir / "fake-install"
        install_dir.mkdir()
    else:
        install_dir = None

    result = install_find_links(
        script, data, ["simple"], dry_run=True, target_dir=install_dir
    )
    assert "Would install simple-3.0" in result.stdout
    assert "Successfully installed" not in result.stdout

    script.assert_not_installed("simple")
Repository: pypa/pip
File: tests/functional/test_new_resolver.py (Python)

Code before:
        "install",
        "--no-cache-dir",
        "--no-index",
        "--find-links",
        script.scratch_path,
        "base",
        expect_error=True,
    )

    message = (
        "Package 'base' requires a different Python: "
        "{}.{}.{} not in '<2'".format(*sys.version_info[:3])
    )
    assert message in result.stderr, str(result)


def test_new_resolver_requires_python_ok_with_python_version_flag(
    script: PipTestEnvironment,
) -> None:
    create_basic_wheel_for_package(
        script,
        "base",
        "0.1.0",
        requires_python="<3",
    )
    result = script.pip(
        "install",
        "--no-cache-dir",
        "--no-index",
        "--find-links",
        script.scratch_path,
        "--python-version=2",
        "--only-binary=:all:",
        "base",
    )

    assert not result.stderr, str(result)


def test_new_resolver_installed(script: PipTestEnvironment) -> None:
    create_basic_wheel_for_package(
        script,
        "base",
        "0.1.0",
        depends=["dep"],
    )
    create_basic_wheel_for_package(
        script,
        "dep",
        "0.1.0",
    )

Review comment (bug):
This currently fails due to https://github.com/pypa/pip/issues/12215, once https://github.com/pypa/pip/pull/12217 is in, I'll merge `main` back into this branch.

Code after:
        "install",
        "--no-cache-dir",
        "--no-index",
        "--find-links",
        script.scratch_path,
        "base",
        expect_error=True,
    )

    message = (
        "Package 'base' requires a different Python: "
        "{}.{}.{} not in '<2'".format(*sys.version_info[:3])
    )
    assert message in result.stderr, str(result)


def test_new_resolver_requires_python_ok_with_python_version_flag(
    script: PipTestEnvironment,
) -> None:
    create_basic_wheel_for_package(
        script,
        "base",
        "0.1.0",
        requires_python="<3",
    )
    result = script.pip(
        "install",
        "--no-cache-dir",
        "--no-index",
        "--find-links",
        script.scratch_path,
        "--dry-run",
        "--python-version=2",
        "--only-binary=:all:",
        "base",
    )

    assert not result.stderr, str(result)


def test_new_resolver_installed(script: PipTestEnvironment) -> None:
    create_basic_wheel_for_package(
        script,
        "base",
        "0.1.0",
        depends=["dep"],
    )
    create_basic_wheel_for_package(
        script,
        "dep",
        "0.1.0",
Repository: pypa/pip
File: src/pip/_internal/commands/debug.py (Python)

Code before:
        suffix = f" (target: {formatted_target})"

    msg = "Compatible tags: {}{}".format(len(tags), suffix)
    logger.info(msg)

    if options.verbose < 1 and len(tags) > tag_limit:
        tags_limited = True
        tags = tags[:tag_limit]
    else:
        tags_limited = False

    with indent_log():
        for tag in tags:
            logger.info(str(tag))

        if tags_limited:
            msg = (
                "...\n[First {tag_limit} tags shown. Pass --verbose to show all.]"
            ).format(tag_limit=tag_limit)
            logger.info(msg)


def ca_bundle_info(config: Configuration) -> str:
    levels = set()
    for key, _ in config.items():  # noqa: PERF102 Configuration has no keys() method.
        levels.add(key.split(".")[0])

    if not levels:
        return "Not specified"

    levels_that_override_global = ["install", "wheel", "download"]
    global_overriding_level = [
        level for level in levels if level in levels_that_override_global
    ]
    if not global_overriding_level:
        return "global"

    if "global" in levels:
        levels.remove("global")
    return ", ".join(levels)


class DebugCommand(Command):
    """
    Display debug information.
    """

    usage = """
      %prog <options>"""
    ignore_require_venv = True


Review comment (suggestion):
I wonder why Ruff does not at least suggest

```python
levels = {key.split(".")[0] for key, _ in config.items()}
```

Code after:
        suffix = f" (target: {formatted_target})"

    msg = "Compatible tags: {}{}".format(len(tags), suffix)
    logger.info(msg)

    if options.verbose < 1 and len(tags) > tag_limit:
        tags_limited = True
        tags = tags[:tag_limit]
    else:
        tags_limited = False

    with indent_log():
        for tag in tags:
            logger.info(str(tag))

        if tags_limited:
            msg = (
                "...\n[First {tag_limit} tags shown. Pass --verbose to show all.]"
            ).format(tag_limit=tag_limit)
            logger.info(msg)


def ca_bundle_info(config: Configuration) -> str:
    # Ruff misidentifies config as a dict.
    # Configuration does not have support the mapping interface.
    levels = {key.split(".", 1)[0] for key, _ in config.items()}  # noqa: PERF102
    if not levels:
        return "Not specified"

    levels_that_override_global = ["install", "wheel", "download"]
    global_overriding_level = [
        level for level in levels if level in levels_that_override_global
    ]
    if not global_overriding_level:
        return "global"

    if "global" in levels:
        levels.remove("global")
    return ", ".join(levels)


class DebugCommand(Command):
    """
    Display debug information.
    """

    usage = """
      %prog <options>"""
    ignore_require_venv = True

    def add_options(self) -> None: