Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dataclasses more liberally throughout the codebase #12571

Merged
merged 18 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions news/f14947e7-deea-4e17-bdc2-dd8dab2a1fa5.trivial.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Convert numerous internal classes to dataclasses for readability and stricter
enforcement of immutability across the codebase. A conservative approach was
taken in selecting which classes to convert. Classes which did not convert
cleanly into a dataclass or were "too complex" (e.g. maintains interconnected
state) were left alone.
27 changes: 11 additions & 16 deletions src/pip/_internal/index/collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import os
import urllib.parse
import urllib.request
from dataclasses import dataclass
from html.parser import HTMLParser
from optparse import Values
from typing import (
Expand Down Expand Up @@ -249,29 +250,23 @@ def parse_links(page: "IndexContent") -> Iterable[Link]:
yield link


@dataclass(frozen=True)
class IndexContent:
"""Represents one response (or page), along with its URL"""
"""Represents one response (or page), along with its URL.

def __init__(
self,
content: bytes,
content_type: str,
encoding: Optional[str],
url: str,
cache_link_parsing: bool = True,
) -> None:
"""
Args:
ichard26 marked this conversation as resolved.
Show resolved Hide resolved
:param encoding: the encoding to decode the given content.
:param url: the URL from which the HTML was downloaded.
:param cache_link_parsing: whether links parsed from this page's url
should be cached. PyPI index urls should
have this set to False, for example.
"""
self.content = content
self.content_type = content_type
self.encoding = encoding
self.url = url
self.cache_link_parsing = cache_link_parsing
"""

content: bytes
content_type: str
encoding: Optional[str]
url: str
cache_link_parsing: bool = True

def __str__(self) -> str:
return redact_auth_from_url(self.url)
Expand Down
15 changes: 4 additions & 11 deletions src/pip/_internal/index/package_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import itertools
import logging
import re
from dataclasses import dataclass
from typing import TYPE_CHECKING, FrozenSet, Iterable, List, Optional, Set, Tuple, Union

from pip._vendor.packaging import specifiers
Expand Down Expand Up @@ -323,23 +324,15 @@ def filter_unallowed_hashes(
return filtered


@dataclass
class CandidatePreferences:
Comment on lines +326 to 327
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The codebase does assign to these attributes from time to time...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sigh


"""
Encapsulates some of the preferences for filtering and sorting
InstallationCandidate objects.
"""

def __init__(
self,
prefer_binary: bool = False,
allow_all_prereleases: bool = False,
) -> None:
"""
:param allow_all_prereleases: Whether to allow all pre-releases.
"""
self.allow_all_prereleases = allow_all_prereleases
self.prefer_binary = prefer_binary
prefer_binary: bool = False
allow_all_prereleases: bool = False


class BestCandidateResult:
Expand Down
6 changes: 4 additions & 2 deletions src/pip/_internal/locations/_sysconfig.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import dataclasses
import logging
import os
import sys
Expand Down Expand Up @@ -192,9 +193,10 @@ def get_scheme(
data=paths["data"],
)
if root is not None:
converted_keys = {}
for key in SCHEME_KEYS:
value = change_root(root, getattr(scheme, key))
setattr(scheme, key, value)
converted_keys[key] = change_root(root, getattr(scheme, key))
scheme = dataclasses.replace(scheme, **converted_keys)
return scheme


Expand Down
29 changes: 12 additions & 17 deletions src/pip/_internal/models/candidate.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,25 @@
from dataclasses import dataclass

from pip._vendor.packaging.version import Version
from pip._vendor.packaging.version import parse as parse_version

from pip._internal.models.link import Link
from pip._internal.utils.models import KeyBasedCompareMixin


class InstallationCandidate(KeyBasedCompareMixin):
@dataclass(frozen=True)
class InstallationCandidate:
"""Represents a potential "candidate" for installation."""

__slots__ = ["name", "version", "link"]

name: str
version: Version
link: Link

def __init__(self, name: str, version: str, link: Link) -> None:
self.name = name
self.version = parse_version(version)
self.link = link

super().__init__(
key=(self.name, self.version, self.link),
defining_class=InstallationCandidate,
)

def __repr__(self) -> str:
return "<InstallationCandidate({!r}, {!r}, {!r})>".format(
self.name,
self.version,
self.link,
)
object.__setattr__(self, "name", name)
object.__setattr__(self, "version", parse_version(version))
object.__setattr__(self, "link", link)

def __str__(self) -> str:
return f"{self.name!r} candidate (version {self.version} at {self.link})"
40 changes: 14 additions & 26 deletions src/pip/_internal/models/direct_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
import json
import re
import urllib.parse
from typing import Any, Dict, Iterable, Optional, Type, TypeVar, Union
from dataclasses import dataclass
from typing import Any, ClassVar, Dict, Iterable, Optional, Type, TypeVar, Union

__all__ = [
"DirectUrl",
Expand Down Expand Up @@ -64,18 +65,13 @@ def _filter_none(**kwargs: Any) -> Dict[str, Any]:
return {k: v for k, v in kwargs.items() if v is not None}


@dataclass
class VcsInfo:
Comment on lines +69 to 70
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted to leave the direct_url classes non-frozen as some tests broke and it would be more consistent with ArchiveInfo which I did not convert (due to non-trivial logic I didn't feel like groking).

name = "vcs_info"
name: ClassVar = "vcs_info"

def __init__(
self,
vcs: str,
commit_id: str,
requested_revision: Optional[str] = None,
) -> None:
self.vcs = vcs
self.requested_revision = requested_revision
self.commit_id = commit_id
vcs: str
commit_id: str
requested_revision: Optional[str] = None

@classmethod
def _from_dict(cls, d: Optional[Dict[str, Any]]) -> Optional["VcsInfo"]:
Expand Down Expand Up @@ -139,14 +135,11 @@ def _to_dict(self) -> Dict[str, Any]:
return _filter_none(hash=self.hash, hashes=self.hashes)


@dataclass
class DirInfo:
name = "dir_info"
name: ClassVar = "dir_info"

def __init__(
self,
editable: bool = False,
) -> None:
self.editable = editable
editable: bool = False

@classmethod
def _from_dict(cls, d: Optional[Dict[str, Any]]) -> Optional["DirInfo"]:
Expand All @@ -161,16 +154,11 @@ def _to_dict(self) -> Dict[str, Any]:
InfoType = Union[ArchiveInfo, DirInfo, VcsInfo]


@dataclass
class DirectUrl:
def __init__(
self,
url: str,
info: InfoType,
subdirectory: Optional[str] = None,
) -> None:
self.url = url
self.info = info
self.subdirectory = subdirectory
url: str
info: InfoType
subdirectory: Optional[str] = None

def _remove_auth_from_netloc(self, netloc: str) -> str:
if "@" not in netloc:
Expand Down
19 changes: 15 additions & 4 deletions src/pip/_internal/models/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
split_auth_from_netloc,
splitext,
)
from pip._internal.utils.models import KeyBasedCompareMixin
from pip._internal.utils.urls import path_to_url, url_to_path

if TYPE_CHECKING:
Expand Down Expand Up @@ -179,7 +178,8 @@ def _ensure_quoted_url(url: str) -> str:
return urllib.parse.urlunparse(result._replace(path=path))


class Link(KeyBasedCompareMixin):
@functools.total_ordering
class Link:
"""Represents a parsed link from a Package Index's simple URL"""

__slots__ = [
Expand Down Expand Up @@ -254,8 +254,6 @@ def __init__(
self.yanked_reason = yanked_reason
self.metadata_file_data = metadata_file_data

super().__init__(key=url, defining_class=Link)

self.cache_link_parsing = cache_link_parsing
self.egg_fragment = self._egg_fragment()

Expand Down Expand Up @@ -375,6 +373,19 @@ def __str__(self) -> str:
def __repr__(self) -> str:
return f"<Link {self}>"

def __hash__(self) -> int:
return hash(self.url)

def __eq__(self, other: Any) -> bool:
if not isinstance(other, Link):
return NotImplemented
return self.url == other.url

def __lt__(self, other: Any) -> bool:
if not isinstance(other, Link):
return NotImplemented
return self.url < other.url

@property
def url(self) -> str:
return self._url
Expand Down
20 changes: 7 additions & 13 deletions src/pip/_internal/models/scheme.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,21 @@
https://docs.python.org/3/install/index.html#alternate-installation.
"""

from dataclasses import dataclass

SCHEME_KEYS = ["platlib", "purelib", "headers", "scripts", "data"]


@dataclass(frozen=True)
class Scheme:
"""A Scheme holds paths which are used as the base directories for
artifacts associated with a Python package.
"""

__slots__ = SCHEME_KEYS

def __init__(
self,
platlib: str,
purelib: str,
headers: str,
scripts: str,
data: str,
) -> None:
self.platlib = platlib
self.purelib = purelib
self.headers = headers
self.scripts = scripts
self.data = data
platlib: str
purelib: str
headers: str
scripts: str
data: str
16 changes: 6 additions & 10 deletions src/pip/_internal/models/search_scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import os
import posixpath
import urllib.parse
from dataclasses import dataclass
from typing import List

from pip._vendor.packaging.utils import canonicalize_name
Expand All @@ -14,6 +15,7 @@
logger = logging.getLogger(__name__)


@dataclass(frozen=True)
class SearchScope:

"""
Expand All @@ -22,6 +24,10 @@ class SearchScope:

__slots__ = ["find_links", "index_urls", "no_index"]

find_links: List[str]
index_urls: List[str]
no_index: bool

@classmethod
def create(
cls,
Expand Down Expand Up @@ -64,16 +70,6 @@ def create(
no_index=no_index,
)

def __init__(
self,
find_links: List[str],
index_urls: List[str],
no_index: bool,
) -> None:
self.find_links = find_links
self.index_urls = index_urls
self.no_index = no_index

def get_formatted_locations(self) -> str:
lines = []
redacted_index_urls = []
Expand Down
2 changes: 2 additions & 0 deletions src/pip/_internal/models/selection_prefs.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from pip._internal.models.format_control import FormatControl


# TODO: This needs Python 3.10's improved slots support for dataclasses
# to be converted into a dataclass.
class SelectionPreferences:
"""
Encapsulates the candidate selection preferences for downloading
Expand Down
14 changes: 8 additions & 6 deletions src/pip/_internal/operations/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import mimetypes
import os
import shutil
from dataclasses import dataclass
from pathlib import Path
from typing import Dict, Iterable, List, Optional

Expand Down Expand Up @@ -80,13 +81,14 @@ def unpack_vcs_link(link: Link, location: str, verbosity: int) -> None:
vcs_backend.unpack(location, url=hide_url(link.url), verbosity=verbosity)


@dataclass
class File:
def __init__(self, path: str, content_type: Optional[str]) -> None:
self.path = path
if content_type is None:
self.content_type = mimetypes.guess_type(path)[0]
else:
self.content_type = content_type
path: str
content_type: Optional[str] = None

def __post_init__(self) -> None:
if self.content_type is None:
self.content_type = mimetypes.guess_type(self.path)[0]


def get_http_url(
Expand Down
Loading
Loading