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

Migrate anonymous Mapping[str, Any] to TypedDicts where their content is fully defined. #716

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
7 changes: 4 additions & 3 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 7 additions & 3 deletions src/poetry/core/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from typing import Any
from typing import List
from typing import Union
from typing import cast

from packaging.utils import canonicalize_name

Expand All @@ -20,6 +21,8 @@

from poetry.core.packages.dependency import Dependency
from poetry.core.packages.dependency_group import DependencyGroup
from poetry.core.packages.project_package import BuildConfigSpec
from poetry.core.packages.project_package import IncludeSpec
from poetry.core.packages.project_package import ProjectPackage
from poetry.core.poetry import Poetry
from poetry.core.spdx.license import License
Expand Down Expand Up @@ -197,9 +200,11 @@ def configure_package(

if "build" in config:
build = config["build"]
if not build:
build = {}
Comment on lines 202 to +204
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
build = config["build"]
if not build:
build = {}
build = config.get("build", {})

?

if not isinstance(build, dict):
build = {"script": build}
package.build_config = build or {}
package.build_config = cast("BuildConfigSpec", build)

if "include" in config:
package.include = []
Expand All @@ -212,8 +217,7 @@ def configure_package(
if formats and not isinstance(formats, list):
formats = [formats]
include["format"] = formats

package.include.append(include)
package.include.append(cast("IncludeSpec", include))

if "exclude" in config:
package.exclude = config["exclude"]
Expand Down
4 changes: 2 additions & 2 deletions src/poetry/core/masonry/builders/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def _module(self) -> Module:
formats = [formats]

if (
formats
len(formats) > 0
Copy link
Member

Choose a reason for hiding this comment

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

Does that make a difference? Isn't the former more Pythonic?

and self.format
and self.format not in formats
and not self._ignore_packages_formats
Expand All @@ -80,7 +80,7 @@ def _module(self) -> Module:
formats = include.get("format", [])

if (
formats
len(formats) > 0
and self.format
and self.format not in formats
and not self._ignore_packages_formats
Expand Down
32 changes: 25 additions & 7 deletions src/poetry/core/packages/project_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,38 @@
import warnings

from typing import TYPE_CHECKING
from typing import Any
from typing import Literal
from typing import Mapping
from typing import Sequence
from typing import TypedDict

from poetry.core.constraints.version import parse_constraint
from poetry.core.packages.package import Package
from poetry.core.packages.utils.utils import create_nested_marker
from poetry.core.version.markers import parse_marker


if TYPE_CHECKING:
from typing_extensions import NotRequired

from poetry.core.constraints.version import Version
from poetry.core.packages.dependency import Dependency

from poetry.core.packages.package import Package
from poetry.core.packages.utils.utils import create_nested_marker
SupportedPackageFormats = Literal["sdist", "wheel"]

BuildConfigSpec = TypedDict(
"BuildConfigSpec",
{"script": NotRequired[str], "generate-setup-file": NotRequired[bool]},
)

class PackageSpec(TypedDict):
include: str
to: str
format: list[SupportedPackageFormats]
Copy link
Member

Choose a reason for hiding this comment

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

Should probably be a Sequence.


class IncludeSpec(TypedDict):
path: str
format: list[SupportedPackageFormats]
Comment on lines +25 to +37
Copy link
Member

Choose a reason for hiding this comment

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

Since these were Mappings before and thus immutable for a reason (see comment in ProjectPackage.__init__), we probably should declare all attributes as ReadOnly.

Copy link
Member

Choose a reason for hiding this comment

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

Should probably be a Sequence.



class ProjectPackage(Package):
Expand All @@ -39,10 +57,10 @@ def __init__(
# Attributes must be immutable for clone() to be safe!
# (For performance reasons, clone only creates a copy instead of a deep copy).

self.build_config: Mapping[str, Any] = {}
self.packages: Sequence[Mapping[str, Any]] = []
self.include: Sequence[Mapping[str, Any]] = []
self.exclude: Sequence[Mapping[str, Any]] = []
self.build_config: BuildConfigSpec = {}
self.packages: Sequence[PackageSpec] = []
self.include: Sequence[IncludeSpec] = []
self.exclude: Sequence[IncludeSpec] = []
self.custom_urls: Mapping[str, str] = {}

if self._python_versions == "*":
Expand Down