diff --git a/src/python/pants/base/specs.py b/src/python/pants/base/specs.py index 77a5418b05b..3b13c97c8ec 100644 --- a/src/python/pants/base/specs.py +++ b/src/python/pants/base/specs.py @@ -414,14 +414,12 @@ class FilesystemSpecs(Collection[FilesystemSpec]): @memoized_property def includes(self) -> Tuple[Union[FilesystemLiteralSpec, FilesystemGlobSpec], ...]: return tuple( - spec - for spec in self.dependencies - if isinstance(spec, (FilesystemGlobSpec, FilesystemLiteralSpec)) + spec for spec in self if isinstance(spec, (FilesystemGlobSpec, FilesystemLiteralSpec)) ) @memoized_property def ignores(self) -> Tuple[FilesystemIgnoreSpec, ...]: - return tuple(spec for spec in self.dependencies if isinstance(spec, FilesystemIgnoreSpec)) + return tuple(spec for spec in self if isinstance(spec, FilesystemIgnoreSpec)) @staticmethod def _generate_path_globs( diff --git a/src/python/pants/engine/collection.py b/src/python/pants/engine/collection.py index 1876d141ac4..b9bd9b35c2e 100644 --- a/src/python/pants/engine/collection.py +++ b/src/python/pants/engine/collection.py @@ -1,14 +1,14 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -from typing import Any, ClassVar, Iterable, Sequence, TypeVar, Union, overload +from typing import Any, ClassVar, Iterable, Tuple, TypeVar, Union, cast, overload from pants.util.ordered_set import FrozenOrderedSet T = TypeVar("T") -class Collection(Sequence[T]): +class Collection(Tuple[T, ...]): """A light newtype around immutable sequences for use with the V2 engine. This should be subclassed when you want to create a distinct collection type, such as: @@ -19,12 +19,9 @@ class Example: class Examples(Collection[Example]): pass - """ - def __init__(self, dependencies: Iterable[T] = ()) -> None: - # TODO: rename to `items`, `elements`, or even make this private. Python consumers should - # not directly access this. - self.dependencies = tuple(dependencies) + N.B: Collection instances are only considered equal if both their types and contents are equal. + """ @overload # noqa: F811 def __getitem__(self, index: int) -> T: @@ -35,23 +32,26 @@ def __getitem__(self, index: slice) -> "Collection[T]": ... def __getitem__(self, index: Union[int, slice]) -> Union[T, "Collection[T]"]: # noqa: F811 + result = super().__getitem__(index) if isinstance(index, int): - return self.dependencies[index] - return self.__class__(self.dependencies[index]) + return cast(T, result) + return self.__class__(cast(Tuple[T, ...], result)) - def __len__(self) -> int: - return len(self.dependencies) + def __eq__(self, other: Any) -> bool: + return type(self) == type(other) and super().__eq__(other) - def __eq__(self, other: Union[Any, "Collection"]) -> bool: - if not isinstance(other, self.__class__): - return NotImplemented - return self.dependencies == other.dependencies + def __ne__(self, other: Any) -> bool: + # We must explicitly override to provide the inverse of _our_ __eq__ and not get the + # inverse of tuple.__eq__. + return not self == other - def __hash__(self) -> int: - return hash(self.dependencies) + # Unlike in Python 2 we must explicitly implement __hash__ since we explicitly implement __eq__ + # per the Python 3 data model. + # See: https://docs.python.org/3/reference/datamodel.html#object.__hash__ + __hash__ = Tuple.__hash__ def __repr__(self) -> str: - return f"{self.__class__.__name__}({list(self.dependencies)})" + return f"{self.__class__.__name__}({list(self)})" # NB: This name is clunky. It would be more appropriate to call it `Set`, but that is claimed by diff --git a/src/python/pants/engine/collection_test.py b/src/python/pants/engine/collection_test.py index 9f6d5a0f12f..40d4ad3661a 100644 --- a/src/python/pants/engine/collection_test.py +++ b/src/python/pants/engine/collection_test.py @@ -46,6 +46,9 @@ def test_collection_reversed() -> None: def test_collection_equality() -> None: + assert () != Collection() + assert Collection() != () + assert Collection([]) == Collection([]) c1 = Collection([1, 2, 3]) assert c1 == Collection([1, 2, 3]) diff --git a/src/rust/engine/src/externs/interface.rs b/src/rust/engine/src/externs/interface.rs index 16fc33d65a6..b1c1fea1d66 100644 --- a/src/rust/engine/src/externs/interface.rs +++ b/src/rust/engine/src/externs/interface.rs @@ -1339,7 +1339,7 @@ fn capture_snapshots( session_ptr: PySession, path_globs_and_root_tuple_wrapper: PyObject, ) -> CPyResult { - let values = externs::project_multi(&path_globs_and_root_tuple_wrapper.into(), "dependencies"); + let values = externs::project_iterable(&path_globs_and_root_tuple_wrapper.into()); let path_globs_and_roots = values .iter() .map(|value| { @@ -1398,7 +1398,7 @@ fn merge_directories( session_ptr: PySession, directories_value: PyObject, ) -> CPyResult { - let digests = externs::project_multi(&directories_value.into(), "dependencies") + let digests = externs::project_iterable(&directories_value.into()) .iter() .map(|v| nodes::lift_digest(v)) .collect::, _>>() diff --git a/src/rust/engine/src/externs/mod.rs b/src/rust/engine/src/externs/mod.rs index 359e4f41a38..8a80a27c54a 100644 --- a/src/rust/engine/src/externs/mod.rs +++ b/src/rust/engine/src/externs/mod.rs @@ -164,7 +164,7 @@ pub fn store_bool(val: bool) -> Value { /// /// Gets an attribute of the given value as the given type. /// -pub fn getattr(value: &Value, field: &str) -> Result +fn getattr(value: &Value, field: &str) -> Result where for<'a> T: FromPyObject<'a>, { @@ -184,6 +184,34 @@ where }) } +/// +/// Collect the Values contained within an outer Python Iterable Value. +/// +fn collect_iterable(value: &Value) -> Result, String> { + let gil = Python::acquire_gil(); + let py = gil.python(); + match value.iter(py) { + Ok(py_iter) => py_iter + .enumerate() + .map(|(i, py_res)| { + py_res.map(Value::from).map_err(|py_err| { + format!( + "Could not iterate {}, failed to extract {}th item: {:?}", + val_to_str(value), + i, + py_err + ) + }) + }) + .collect(), + Err(py_err) => Err(format!( + "Could not iterate {}: {:?}", + val_to_str(value), + py_err + )), + } +} + /// /// Pulls out the value specified by the field name from a given Value /// @@ -191,6 +219,10 @@ pub fn project_ignoring_type(value: &Value, field: &str) -> Value { getattr(value, field).unwrap() } +pub fn project_iterable(value: &Value) -> Vec { + collect_iterable(value).unwrap() +} + pub fn project_multi(value: &Value, field: &str) -> Vec { getattr(value, field).unwrap() } diff --git a/src/rust/engine/src/intrinsics.rs b/src/rust/engine/src/intrinsics.rs index e52248eab04..d5e4214eba5 100644 --- a/src/rust/engine/src/intrinsics.rs +++ b/src/rust/engine/src/intrinsics.rs @@ -314,7 +314,7 @@ fn create_digest_to_digest( context: Context, args: Vec, ) -> BoxFuture<'static, NodeResult> { - let file_values = externs::project_multi(&args[0], "dependencies"); + let file_values = externs::project_iterable(&args[0]); let digests: Vec<_> = file_values .iter() .map(|file| {