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

chore(packages): remove type errors #339

Closed
Closed
Show file tree
Hide file tree
Changes from all 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
3 changes: 0 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,6 @@ module = [
'poetry.core.masonry.builders.builder',
'poetry.core.masonry.builders.sdist',
'poetry.core.masonry.builders.wheel',
'poetry.core.packages.utils.utils',
'poetry.core.packages.dependency',
'poetry.core.packages.package',
'poetry.core.semver.version_union',
'poetry.core.toml.exceptions',
'poetry.core.toml.file',
Expand Down
52 changes: 24 additions & 28 deletions src/poetry/core/packages/dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from typing import TYPE_CHECKING
from typing import Any
from typing import Iterable
from typing import cast

from poetry.core.packages.constraints import (
parse_constraint as parse_generic_constraint,
Expand Down Expand Up @@ -55,8 +56,8 @@ def __init__(
features=extras,
)

self._constraint: str | VersionConstraint | None = None
self._pretty_constraint: str | None = None
self._constraint: VersionConstraint
self._pretty_constraint: str
self.set_constraint(constraint=constraint)

self._optional = optional
Expand All @@ -78,17 +79,17 @@ def __init__(

self._python_versions = "*"
self._python_constraint = parse_constraint("*")
self._transitive_python_versions = None
self._transitive_python_versions: str | None = None
self._transitive_python_constraint: VersionConstraint | None = None
self._transitive_marker = None
self._transitive_marker: BaseMarker | None = None
self._extras = frozenset(extras or [])

self._in_extras = []
self._in_extras: list[str] = []

self._activated = not self._optional

self.is_root = False
self._marker = AnyMarker()
self._marker: BaseMarker = AnyMarker()
self.source_name = None

@property
Expand Down Expand Up @@ -269,9 +270,9 @@ def base_pep_508_name(self) -> str:
# no functional difference with the logic in the else branch.
requirement += f" ({str(constraint)})"
else:
constraints = self.pretty_constraint.split(",")
constraints = [parse_constraint(c) for c in constraints]
constraints = ",".join(str(c) for c in constraints)
constraints = ",".join(
str(parse_constraint(c)) for c in self.pretty_constraint.split(",")
)
requirement += f" ({constraints})"
elif isinstance(constraint, Version):
requirement += f" (=={constraint.text})"
Expand Down Expand Up @@ -349,8 +350,8 @@ def to_pep_508(self, with_extras: bool = True) -> str:
requirement += " "

if len(markers) > 1:
markers = " and ".join(f"({m})" for m in markers)
requirement += f"; {markers}"
marker_str = " and ".join(f"({m})" for m in markers)
requirement += f"; {marker_str}"
else:
requirement += f"; {markers[0]}"

Expand All @@ -366,29 +367,23 @@ def _create_nested_marker(
from poetry.core.semver.version_union import VersionUnion

if isinstance(constraint, (MultiConstraint, UnionConstraint)):
parts = []
multi_parts = []
for c in constraint.constraints:
multi = False
if isinstance(c, (MultiConstraint, UnionConstraint)):
multi = True

parts.append((multi, self._create_nested_marker(name, c)))
multi = isinstance(c, (MultiConstraint, UnionConstraint))
multi_parts.append((multi, self._create_nested_marker(name, c)))

glue = " and "
if isinstance(constraint, UnionConstraint):
parts = [f"({part[1]})" if part[0] else part[1] for part in parts]
parts = [f"({part[1]})" if part[0] else part[1] for part in multi_parts]
glue = " or "
else:
parts = [part[1] for part in parts]
parts = [part[1] for part in multi_parts]

marker = glue.join(parts)
elif isinstance(constraint, Constraint):
marker = f'{name} {constraint.operator} "{constraint.version}"'
elif isinstance(constraint, VersionUnion):
parts = []
for c in constraint.ranges:
parts.append(self._create_nested_marker(name, c))

parts = [self._create_nested_marker(name, c) for c in constraint.ranges]
glue = " or "
parts = [f"({part})" for part in parts]

Expand All @@ -412,7 +407,7 @@ def _create_nested_marker(
if not constraint.include_min:
op = ">"

version = constraint.min.text
version: Version | str = constraint.min.text
Copy link
Contributor

Choose a reason for hiding this comment

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

or I think this can always be a string (without needing annotation) if you set version = constraint.max.text a few lines later

if constraint.max is not None:
max_name = name
if constraint.max.precision >= 3 and name == "python_version":
Expand Down Expand Up @@ -517,15 +512,15 @@ def create_from_pep_508(
req = Requirement(name)

name = req.name
path = os.path.normpath(os.path.abspath(name))
link = None

if is_url(name):
link = Link(name)
elif req.url:
link = Link(req.url)
else:
p, extras = strip_extras(path)
path_str = os.path.normpath(os.path.abspath(name))
p, extras = strip_extras(path_str)
if os.path.isdir(p) and (os.path.sep in name or name.startswith(".")):

if not is_installable_dir(p):
Expand All @@ -538,6 +533,7 @@ def create_from_pep_508(
link = Link(path_to_url(p))

# it's a local file, dir, or url
dep: Dependency | None
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: initialize the value to None here, rather than later.

if link:
is_file_uri = link.scheme == "file"
is_relative_uri = is_file_uri and re.search(r"\.\./", link.url)
Expand All @@ -558,7 +554,7 @@ def create_from_pep_508(
name = m.group("name")
version = m.group("ver")

name = req.name or link.egg_fragment
name = cast(str, req.name or link.egg_fragment)
Copy link
Contributor

Choose a reason for hiding this comment

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

it's not clear what the intention of the (original) code is here: the name extracted only three lines previously is discarded here. So something has gone wrong and it would be good to take the opportunity to fix it.

My guess is that this whole fragment can be discarded (anyway that doesn't break any tests).

Copy link
Contributor

@dimbleby dimbleby May 15, 2022

Choose a reason for hiding this comment

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

(and if that's right then there are no users of egg_fragment left so maybe that can be discarded too)

dep = None

if link.scheme.startswith("git+"):
Expand Down Expand Up @@ -599,11 +595,11 @@ def create_from_pep_508(
if version:
dep._constraint = parse_constraint(version)
else:
constraint: VersionConstraint | str
if req.pretty_constraint:
constraint = req.constraint
else:
constraint = "*"

dep = Dependency(name, constraint, extras=req.extras)

if req.marker:
Expand Down
4 changes: 1 addition & 3 deletions src/poetry/core/packages/directory_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,7 @@ def with_constraint(
extras=list(self._extras),
)

new._constraint = constraint
new._pretty_constraint = str(constraint)

new.set_constraint(constraint)
Copy link
Member Author

Choose a reason for hiding this comment

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

before, this allowed constraint to be a str, although it seems the intention was for set_constraint to handle normalizing to VersionConstraint (as well as setting _pretty_constraint)

def set_constraint(self, constraint: str | VersionConstraint) -> None:

new.is_root = self.is_root
new.python_versions = self.python_versions
new.marker = self.marker
Expand Down
4 changes: 1 addition & 3 deletions src/poetry/core/packages/file_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ def with_constraint(self, constraint: str | VersionConstraint) -> FileDependency
extras=list(self._extras),
)

new._constraint = constraint
new._pretty_constraint = str(constraint)

new.set_constraint(constraint)
new.is_root = self.is_root
new.python_versions = self.python_versions
new.marker = self.marker
Expand Down
46 changes: 25 additions & 21 deletions src/poetry/core/packages/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
from typing import TYPE_CHECKING
from typing import Collection
from typing import Iterable
from typing import Iterator
from typing import TypeVar
from typing import cast

from poetry.core.packages.dependency_group import MAIN_GROUP
from poetry.core.packages.specification import PackageSpecification
Expand Down Expand Up @@ -83,18 +85,18 @@ def __init__(

self.description = ""

self._authors = []
self._maintainers = []
self._authors: list[str] = []
self._maintainers: list[str] = []

self.homepage: str | None = None
self.repository_url: str | None = None
self.documentation_url: str | None = None
self.keywords: list[str] = []
self._license = None
self._license: License | None = None
self.readmes: tuple[Path, ...] = ()

self.extras = {}
self.requires_extras = []
self.extras: dict[str, list[str | Dependency]] = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

pretty sure this should be dict[str, list[Dependency]]

self.requires_extras: list[str] = []

self._dependency_groups: dict[str, DependencyGroup] = {}

Expand All @@ -103,7 +105,7 @@ def __init__(
self.files: list[dict[str, str]] = []
self.optional = False

self.classifiers = []
self.classifiers: list[str] = []

self._python_versions = "*"
self._python_constraint = parse_constraint("*")
Expand Down Expand Up @@ -151,12 +153,13 @@ def full_pretty_version(self) -> str:
if self.source_type not in ["hg", "git"]:
return self._pretty_version

ref: str | None
if self.source_resolved_reference and len(self.source_resolved_reference) == 40:
ref = self.source_resolved_reference[0:7]
return f"{self._pretty_version} {ref}"

# if source reference is a sha1 hash -- truncate
if len(self.source_reference) == 40:
if self.source_reference and len(self.source_reference) == 40:
return f"{self._pretty_version} {self.source_reference[0:7]}"

ref = self._source_resolved_reference or self._source_reference
Expand All @@ -167,23 +170,23 @@ def authors(self) -> list[str]:
return self._authors

@property
def author_name(self) -> str:
def author_name(self) -> str | None:
return self._get_author()["name"]

@property
def author_email(self) -> str:
def author_email(self) -> str | None:
return self._get_author()["email"]

@property
def maintainers(self) -> list[str]:
return self._maintainers

@property
def maintainer_name(self) -> str:
def maintainer_name(self) -> str | None:
return self._get_maintainer()["name"]

@property
def maintainer_email(self) -> str:
def maintainer_email(self) -> str | None:
return self._get_maintainer()["email"]

@property
Expand Down Expand Up @@ -345,7 +348,7 @@ def urls(self) -> dict[str, str]:
return urls

@property
def readme(self) -> Path:
def readme(self) -> Path | None:
import warnings

warnings.warn(
Expand Down Expand Up @@ -458,10 +461,11 @@ def to_dependency(self) -> Dependency:
from poetry.core.packages.url_dependency import URLDependency
from poetry.core.packages.vcs_dependency import VCSDependency

dep: Dependency
if self.source_type == "directory":
dep = DirectoryDependency(
self._name,
Path(self._source_url),
Path(cast(str, self._source_url)),
groups=list(self._dependency_groups.keys()),
optional=self.optional,
base=self.root_dir,
Expand All @@ -471,7 +475,7 @@ def to_dependency(self) -> Dependency:
elif self.source_type == "file":
dep = FileDependency(
self._name,
Path(self._source_url),
Path(cast(str, self._source_url)),
groups=list(self._dependency_groups.keys()),
optional=self.optional,
base=self.root_dir,
Expand All @@ -480,7 +484,7 @@ def to_dependency(self) -> Dependency:
elif self.source_type == "url":
dep = URLDependency(
self._name,
self._source_url,
cast(str, self._source_url),
groups=list(self._dependency_groups.keys()),
optional=self.optional,
extras=self.features,
Expand All @@ -489,7 +493,7 @@ def to_dependency(self) -> Dependency:
dep = VCSDependency(
self._name,
self.source_type,
self.source_url,
cast(str, self.source_url),
rev=self.source_reference,
resolved_rev=self.source_resolved_reference,
directory=self.source_subdirectory,
Expand All @@ -513,7 +517,7 @@ def to_dependency(self) -> Dependency:
return dep.with_constraint(self._version)

@contextmanager
def with_python_versions(self, python_versions: str) -> None:
def with_python_versions(self, python_versions: str) -> Iterator[None]:
original_python_versions = self.python_versions

self.python_versions = python_versions
Expand All @@ -532,15 +536,15 @@ def with_features(self, features: Iterable[str]) -> Package:
def without_features(self) -> Package:
return self.with_features([])

def clone(self) -> Package:
def clone(self: T) -> T:
clone = self.__class__(self.pretty_name, self.version)
clone.__dict__ = copy.deepcopy(self.__dict__)
return clone

def __hash__(self) -> int:
return super().__hash__() ^ hash(self._version)

def __eq__(self, other: Package) -> bool:
def __eq__(self, other: object) -> bool:
if not isinstance(other, Package):
return NotImplemented

Expand All @@ -567,5 +571,5 @@ def __repr__(self) -> str:
f"source_resolved_reference={repr(self._source_resolved_reference)}"
)

args = ", ".join(args)
return f"Package({args})"
args_str = ", ".join(args)
return f"Package({args_str})"
4 changes: 1 addition & 3 deletions src/poetry/core/packages/url_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ def with_constraint(self, constraint: str | VersionConstraint) -> URLDependency:
extras=list(self._extras),
)

new._constraint = constraint
new._pretty_constraint = str(constraint)

new.set_constraint(constraint)
new.is_root = self.is_root
new.python_versions = self.python_versions
new.marker = self.marker
Expand Down
Loading