Skip to content

Commit

Permalink
remove PrimitiveField
Browse files Browse the repository at this point in the history 
fix invalid type error method

fix fmt
  • Loading branch information
@cosmicexplorer
cosmicexplorer committed Oct 15, 2020
1 parent 02c8d40 commit 659c5be
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 117 deletions.
14 changes: 4 additions & 10 deletions src/python/pants/backend/pants_info/list_target_types.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
Field,
FloatField,
IntField,
PrimitiveField,
RegisteredTargetTypes,
ScalarField,
SequenceField,
Expand Down Expand Up @@ -108,7 +107,7 @@ def create(cls, field: Type[Field]) -> "FieldInfo":
# requiring the Field author to rewrite the docstring.
#
# However, if the original `Field` author did not define docstring, then this means we
# would typically fall back to the docstring for `AsyncField`, `PrimitiveField`, or a
# would typically fall back to the docstring for `AsyncField`, `Field`, or a
# helper class like `StringField`. This is a quirk of this heuristic and it's not
# intentional since these core `Field` types have documentation oriented to the custom
# `Field` author and not the end user filling in fields in a BUILD file target.
Expand All @@ -119,7 +118,7 @@ def create(cls, field: Type[Field]) -> "FieldInfo":
ignored_ancestors={
*Field.mro(),
AsyncField,
PrimitiveField,
Field,
BoolField,
DictStringToStringField,
DictStringToStringSequenceField,
Expand All @@ -134,12 +133,7 @@ def create(cls, field: Type[Field]) -> "FieldInfo":
TriBoolField,
},
)
if issubclass(field, PrimitiveField):
raw_value_type = get_type_hints(field.compute_value)["raw_value"]
elif issubclass(field, AsyncField):
raw_value_type = get_type_hints(field.compute_value)["raw_value"]
else:
raw_value_type = get_type_hints(field.__init__)["raw_value"]
raw_value_type = get_type_hints(field.compute_value)["raw_value"]
type_hint = pretty_print_type_hint(raw_value_type)

# Check if the field only allows for certain choices.
Expand All @@ -154,7 +148,7 @@ def create(cls, field: Type[Field]) -> "FieldInfo":
if field.required:
# We hackily remove `None` as a valid option for the field when it's required. This
# greatly simplifies Field definitions because it means that they don't need to
# override the type hints for `PrimitiveField.compute_value()` and
# override the type hints for `Field.compute_value()` and
# `AsyncField.compute_value()` to indicate that `None` is an invalid type.
type_hint = type_hint.replace(" | None", "")

Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/backend/python/target_types.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@
BoolField,
Dependencies,
DictStringToStringSequenceField,
Field,
InjectDependenciesRequest,
InjectedDependencies,
IntField,
InvalidFieldException,
InvalidFieldTypeException,
PrimitiveField,
ProvidesField,
ScalarField,
Sources,
Expand Down Expand Up @@ -439,7 +439,7 @@ def format_invalid_requirement_string_error(
)


class PythonRequirementsField(PrimitiveField[Tuple[Requirement, ...]]):
class PythonRequirementsField(Field[Tuple[Requirement, ...]]):
"""A sequence of pip-style requirement strings, e.g. ['foo==1.8', 'bar<=3 ;
python_version<'3']."""

Expand All @@ -452,8 +452,8 @@ def compute_value(
) -> Tuple[Requirement, ...]:
value = super().compute_value(raw_value, address=address)

def invalid_type_error():
raise InvalidFieldTypeException(
def invalid_type_error() -> InvalidFieldTypeException:
return InvalidFieldTypeException(
address,
cls.alias,
value,
Expand Down
179 changes: 87 additions & 92 deletions src/python/pants/engine/target.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import dataclasses
import itertools
import os.path
from abc import ABC, ABCMeta, abstractmethod
from abc import ABC, ABCMeta
from dataclasses import dataclass
from enum import Enum
from pathlib import PurePath
Expand Down Expand Up @@ -49,83 +49,30 @@
# Core Field abstractions
# -----------------------------------------------------------------------------------------------

# Type alias to express the intent that the type should be immutable and hashable. There's nothing
# to enforce immutability, outside of convention. Maybe we could develop a MyPy plugin?
ImmutableValue = Hashable

_DefaultBase = TypeVar("_DefaultBase", bound=Optional[ImmutableValue])


# This @dataclass declaration is necessary for the `value` field to be propagated to the __hash__ of
# all subclasses, but MyPy doesn't recognize generic abstract dataclasses yet.
@dataclass(unsafe_hash=True) # type: ignore[misc]
class Field(Generic[_DefaultBase], metaclass=ABCMeta):
value: _DefaultBase
# Subclasses must define these.
alias: ClassVar[str]
default: ClassVar[_DefaultBase]
# Subclasses may define these.
required: ClassVar[bool] = False
deprecated_removal_version: ClassVar[Optional[str]] = None
deprecated_removal_hint: ClassVar[Optional[str]] = None

# NB: We still expect `PrimitiveField` and `AsyncField` to define their own constructors. This
# is only implemented so that we have a common deprecation mechanism, and to ensure that all
# subclasses have this exact type signature for their constructor. Normally we would rely on the
# generated __init__ by declaring `address` as a field, but we don't want that on *every* Field
# subclass.
@abstractmethod
def __init__(self, raw_value: Optional[Any], *, address: Address) -> None:
if self.deprecated_removal_version and address.is_base_target and raw_value is not None:
if not self.deprecated_removal_hint:
raise ValueError(
f"You specified `deprecated_removal_version` for {self.__class__}, but not "
"the class property `deprecated_removal_hint`."
)
warn_or_error(
removal_version=self.deprecated_removal_version,
deprecated_entity_description=f"the {repr(self.alias)} field",
hint=(
f"Using the `{self.alias}` field in the target {address}. "
f"{self.deprecated_removal_hint}"
),
)

@classmethod
def compute_value(
cls,
raw_value: Optional[Any],
*,
address: Address,
) -> Optional[Any]:
"""Convert the `raw_value` into `self.value`.
You should perform any optional validation and/or hydration here. For example, you may want
to check that an integer is > 0 or convert an Iterable[str] to List[str].
The resulting value must be hashable (and should be immutable).
"""
if raw_value is None:
if cls.required:
raise RequiredFieldMissingException(address, cls.alias)
return cls.default
return raw_value
# TODO: There's nothing to enforce immutability, outside of convention. Maybe we could develop a
# MyPy plugin?
_DefaultBase = TypeVar("_DefaultBase", bound=Optional[Hashable])


@frozen_after_init
@dataclass(unsafe_hash=True)
class PrimitiveField(Field[_DefaultBase], metaclass=ABCMeta):
class Field(Generic[_DefaultBase], metaclass=ABCMeta):
"""A Field that does not need the engine in order to be hydrated.
The majority of fields should use subclasses of `PrimitiveField`, e.g. use `BoolField`,
`StringField`, or `StringSequenceField`. These subclasses will provide sane type hints and
The majority of fields should use subclasses of `Field`, e.g. use `BoolField`,
`StringField`, or `StringSequenceField`. These subclasses will provide sensible type hints and
hydration/validation automatically.
If you are directly subclassing `PrimitiveField`, you should likely override `compute_value()`
to perform any custom hydration and/or validation, such as converting unhashable types to
hashable types or checking for banned values. The returned value must be hashable
(and should be immutable) so that this Field may be used by the V2 engine. This means, for
example, using tuples rather than lists and using `FrozenOrderedSet` rather than `set`.
If you are directly subclassing `Field`, you will want to give it a type parameter to set the
type of `value`. These direct subclasses should also likely override `compute_value()` to
perform any custom hydration and/or validation, such as converting unhashable types to hashable
types or checking for banned values. The returned value must be hashable (and should be
immutable) so that this Field may be used by the V2 engine. This means, for example, using
tuples rather than lists and using `FrozenOrderedSet` rather than `set`.
Direct subclasses of `Field` should also override the type hint for `raw_value`, as that is used
to generate documentation, e.g. for `./pants target-types`. If the field is required, do not use
`Optional` for the type hint of `raw_value`.
All hydration and/or validation happens eagerly in the constructor. If the
hydration is particularly expensive, use `AsyncField` instead to get the benefits of the
Expand All @@ -138,8 +85,8 @@ class PrimitiveField(Field[_DefaultBase], metaclass=ABCMeta):
Example:
# NB: Really, this should subclass IntField. We only use PrimitiveField as an example.
class Timeout(PrimitiveField[Optional[int]]):
# NB: Really, this should subclass IntField. We only use Field as an example.
class Timeout(Field[Optional[int]]):
alias = "timeout"
default = None
Expand All @@ -154,15 +101,67 @@ def compute_value(cls, raw_value: Optional[int], *, address: Address) -> Optiona
return value_or_default
"""

# This exists on every Field subclass.
value: _DefaultBase
# Subclasses must define these.
alias: ClassVar[str]
default: ClassVar[_DefaultBase]
# Subclasses may define these.
# TODO: enforce that `raw_value` and `value` are *not* Optional if `required = True`, possibly
# via mixin or decorator?
required: ClassVar[bool] = False
deprecated_removal_version: ClassVar[Optional[str]] = None
deprecated_removal_hint: ClassVar[Optional[str]] = None

# This method is marked @final so that we have a common deprecation mechanism, and to ensure
# that all subclasses have this exact type signature for their constructor.
@final
def __init__(self, raw_value: Optional[Any], *, address: Address) -> None:
self._check_deprecated(raw_value, address)
# NB: We neither store the `address` or `raw_value` as attributes on this dataclass:
# * Don't store `raw_value` because it very often is mutable and/or unhashable, which means
# this Field could not be passed around in the engine.
# * Don't store `address` to avoid the cost in memory of storing `Address` on every single
# field encountered by Pants in a run.
self.value = cast(_DefaultBase, self.compute_value(raw_value, address=address))

@classmethod
def compute_value(
cls,
raw_value: Optional[Any],
*,
address: Address,
) -> Optional[Any]:
"""Convert the `raw_value` into `self.value`.
You should perform any optional validation and/or hydration here. For example, you may want
to check that an integer is > 0 or convert an Iterable[str] to List[str].
The resulting value must be hashable (and should be immutable).
"""
if raw_value is None:
if cls.required:
raise RequiredFieldMissingException(address, cls.alias)
return cls.default
return raw_value

@classmethod
def _check_deprecated(cls, raw_value: Optional[Any], address: Address) -> None:
if cls.deprecated_removal_version and address.is_base_target and raw_value is not None:
if not cls.deprecated_removal_hint:
raise ValueError(
f"You specified `deprecated_removal_version` for {cls}, but not "
"the class property `deprecated_removal_hint`."
)
warn_or_error(
removal_version=cls.deprecated_removal_version,
deprecated_entity_description=f"the {repr(cls.alias)} field",
hint=(
f"Using the `{cls.alias}` field in the target {address}. "
f"{cls.deprecated_removal_hint}"
),
)

def __repr__(self) -> str:
return (
f"{self.__class__}(alias={repr(self.alias)}, value={repr(self.value)}, "
Expand Down Expand Up @@ -234,20 +233,15 @@ def rules():

address: Address

@final
# We cheat here by ignoring the @final declaration *and* @frozen_after_init. We don't want to
# generalize this further, because we want to avoid having `Field` subclasses start adding
# arbitrary fields.
@final # type: ignore[misc]
def __init__(self, raw_value: Optional[Any], *, address: Address) -> None:
super().__init__(raw_value, address=address)
self._unfreeze_instance() # type: ignore[attr-defined]
self.address = address
self.value = cast(_DefaultBase, self.compute_value(raw_value, address=address))

def __repr__(self) -> str:
return (
f"{self.__class__}(alias={repr(self.alias)}, "
f"value={repr(self.value)}, default={repr(self.default)})"
)

def __str__(self) -> str:
return f"{self.alias}={self.value}"
self._freeze_instance() # type: ignore[attr-defined]


# -----------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -748,7 +742,7 @@ def generate_subtarget(

generated_target_fields = {}
for field in base_target.field_values.values():
value: Optional[ImmutableValue]
value: Optional[Hashable]
if isinstance(field, Sources):
if not bool(matches_filespec(field.filespec, paths=[full_file_name])):
raise ValueError(
Expand Down Expand Up @@ -1006,10 +1000,10 @@ def __init__(
# Field templates
# -----------------------------------------------------------------------------------------------

T = TypeVar("T", bound=ImmutableValue)
T = TypeVar("T", bound=Hashable)


class ScalarField(PrimitiveField[Optional[T]], metaclass=ABCMeta):
class ScalarField(Field[Optional[T]], metaclass=ABCMeta):
"""A field with a scalar value (vs. a compound value like a sequence or dict).
Subclasses must define the class properties `expected_type` and `expected_type_description`.
Expand Down Expand Up @@ -1045,7 +1039,7 @@ def compute_value(cls, raw_value: Optional[Any], *, address: Address) -> Optiona
return value_or_default


class TriBoolField(PrimitiveField[Optional[bool]], metaclass=ABCMeta):
class TriBoolField(ScalarField[bool], metaclass=ABCMeta):
"""A field whose value is a boolean.
If subclasses do not set the class property `required = True` or `default`, the value will
Expand All @@ -1056,7 +1050,8 @@ class ZipSafe(BoolField):
default = True
"""

default: ClassVar[Optional[bool]] = None
expected_type = bool
expected_type_description = "a boolean"

@classmethod
def compute_value(cls, raw_value: Optional[bool], *, address: Address) -> Optional[bool]:
Expand Down Expand Up @@ -1123,7 +1118,7 @@ def compute_value(cls, raw_value: Optional[str], *, address: Address) -> Optiona
return value_or_default


class SequenceField(PrimitiveField[Optional[Tuple[T, ...]]], metaclass=ABCMeta):
class SequenceField(Field[Optional[Tuple[T, ...]]], metaclass=ABCMeta):
"""A field whose value is a homogeneous sequence.
Subclasses must define the class properties `expected_element_type` and
Expand Down Expand Up @@ -1199,7 +1194,7 @@ def compute_value(
return super().compute_value(raw_value, address=address)


class DictStringToStringField(PrimitiveField[Optional[FrozenDict[str, str]]], metaclass=ABCMeta):
class DictStringToStringField(Field[Optional[FrozenDict[str, str]]], metaclass=ABCMeta):
default: ClassVar[Optional[FrozenDict[str, str]]] = None

@classmethod
Expand All @@ -1223,7 +1218,7 @@ def invalid_type_exception():


class DictStringToStringSequenceField(
PrimitiveField[Optional[FrozenDict[str, Tuple[str, ...]]]],
Field[Optional[FrozenDict[str, Tuple[str, ...]]]],
metaclass=ABCMeta,
):
default: ClassVar[Optional[FrozenDict[str, Tuple[str, ...]]]] = None
Expand Down Expand Up @@ -1738,7 +1733,7 @@ class DescriptionField(StringField):

# TODO: figure out what support looks like for this with the Target API. The expected value is an
# Artifact, there is no common Artifact interface.
class ProvidesField(PrimitiveField[Optional[Any]]):
class ProvidesField(Field[Optional[Any]]):
"""An `artifact`, such as `setup_py`, that describes how to represent this target to the outside
world."""

Expand Down
Loading

0 comments on commit 659c5be

Please sign in to comment.