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

Updates __eq__ implementation on the data types #445

Merged
merged 26 commits into from
Dec 19, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7cd21b5
fix: PackageVersion.__eq__
Karrenbelt Nov 16, 2022
064b845
fix: __eq__ PublicId, PackageId & Dependency
Karrenbelt Nov 16, 2022
cdbb001
fix: check_dependencies -> .without_hash()
Karrenbelt Nov 16, 2022
3b27e78
chore: linters
Karrenbelt Nov 16, 2022
0b33f7a
fix: remove _package_hash from PublicId.__eq__
Karrenbelt Nov 17, 2022
77474ff
Revert "fix: check_dependencies -> .without_hash()"
Karrenbelt Nov 17, 2022
36cf832
fix: deflection in __lt__ method
Karrenbelt Nov 17, 2022
05dbb00
fix: add functools.total_ordering to PublicId and PackageId
Karrenbelt Nov 17, 2022
f564c43
chore: linters
Karrenbelt Nov 17, 2022
09f7179
fix: error message in code and type in test
Karrenbelt Nov 17, 2022
4c118f4
tests: package_version_strategy
Karrenbelt Nov 17, 2022
75549e5
fix: test_self_type_comparison
Karrenbelt Nov 17, 2022
45e1c7a
chore: linters
Karrenbelt Nov 17, 2022
85c0c74
fix: delegate comparison to PackageVersionLike
Karrenbelt Nov 17, 2022
3b5a411
fix: remove `sorted` from DependencyNotFound.print_error
Karrenbelt Nov 17, 2022
64f19d0
fix: remove `sorted` from DependencyTree
Karrenbelt Nov 17, 2022
15b128c
fix: test_package_version_lt
Karrenbelt Nov 17, 2022
4784ac1
fix: remove sorted() from queue initialization
Karrenbelt Dec 17, 2022
24d3fd1
Merge branch 'tests/test_data_types' into fix/data_types__eq__
Karrenbelt Dec 17, 2022
f269b13
Merge branch 'fix/data_types__eq__' into fix/data_types__lt__
Karrenbelt Dec 17, 2022
779c951
Merge branch 'fix/data_types__lt__' into fix/delegate_to_PackageVersi…
Karrenbelt Dec 17, 2022
97e612a
tests: fix test_type_comparison
Karrenbelt Dec 18, 2022
1e08814
chore: linters
Karrenbelt Dec 18, 2022
f4aea8d
fix: sorted(roots, key=str)
Karrenbelt Dec 18, 2022
9276d3a
Merge pull request #448 from valory-xyz/fix/delegate_to_PackageVersio…
DavidMinarsch Dec 19, 2022
7281b27
Merge pull request #447 from valory-xyz/fix/data_types__lt__
DavidMinarsch Dec 19, 2022
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
40 changes: 17 additions & 23 deletions aea/configurations/data_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ class PackageVersion:

_version: PackageVersionLike

__slots__ = ("_version",)

Comment on lines +88 to +89
Copy link
Author

Choose a reason for hiding this comment

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

such that a generic solution could be implemented that is equal for all classes in this module

def __init__(self, version_like: PackageVersionLike) -> None:
"""
Initialize a package version.
Expand Down Expand Up @@ -114,7 +116,9 @@ def __str__(self) -> str:

def __eq__(self, other: Any) -> bool:
"""Check equality."""
return isinstance(other, PackageVersion) and self._version == other._version
if not isinstance(other, self.__class__):
return NotImplemented # Delegate comparison to the other instance's __eq__.
return all(getattr(self, s) == getattr(other, s) for s in self.__slots__)

def __lt__(self, other: Any) -> bool:
"""Compare with another object."""
Expand Down Expand Up @@ -451,13 +455,10 @@ def __repr__(self) -> str:
return f"<{self}>"

def __eq__(self, other: Any) -> bool:
"""Compare with another object."""
return (
isinstance(other, PublicId)
and self.author == other.author
and self.name == other.name
and self.version == other.version
)
"""Check equality."""
if not isinstance(other, self.__class__):
return NotImplemented # Delegate comparison to the other instance's __eq__.
return all(getattr(self, s) == getattr(other, s) for s in self.__slots__[:-1])
Comment on lines +453 to +456
Copy link
Author

@Karrenbelt Karrenbelt Nov 17, 2022

Choose a reason for hiding this comment

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

including the _package_hash will affect the functionality in the following 4 places, which is why I chose not to include it now (it was not included in the original comparison either). We may choose to open an issue on this if we want to change this later.

  • eject

    open-aea/aea/cli/eject.py

    Lines 204 to 206 in 77474ff

    reverse_reachable_dependencies = reachable_nodes(
    reverse_dependencies, {package_id.without_hash()}
    )
  • remove

    open-aea/aea/cli/remove.py

    Lines 193 to 226 in 77474ff

    def get_item_dependencies_with_reverse_dependencies(
    self, item: PackageConfiguration, package_id: Optional[PackageId] = None
    ) -> Dict[PackageId, Set[PackageId]]:
    """
    Get item dependencies.
    It's recursive and provides all the sub dependencies.
    :param item: the item package configuration
    :param package_id: the package id.
    :return: dict with PackageId: and set of PackageIds that uses this package
    """
    result: defaultdict = defaultdict(set)
    for dep_package_id in self._get_item_requirements(
    item, self._ignore_non_vendor
    ):
    if package_id is None:
    _ = result[dep_package_id.without_hash()] # init default dict value
    else:
    result[dep_package_id.without_hash()].add(package_id.without_hash())
    if not self.is_present_in_agent_config(
    dep_package_id.without_hash()
    ): # pragma: nocover
    continue
    dep_item = self.get_item_config(dep_package_id)
    for item_key, deps in self.get_item_dependencies_with_reverse_dependencies(
    dep_item, dep_package_id
    ).items():
    result[item_key.without_hash()] = result[item_key].union(deps)
    return result
  • check_packages
    def _add_package_type(package_type: PackageType, public_id_str: str) -> PackageId:
    return PackageId(package_type, PublicId.from_str(public_id_str))
  • registry/add
    def fetch_package(obj_type: str, public_id: PublicId, cwd: str, dest: str) -> Path:
    """
    Fetch a package (connection/contract/protocol/skill) from Registry.
    :param obj_type: str type of object you want to fetch:
    'connection', 'protocol', 'skill'
    :param public_id: str public ID of object.
    :param cwd: str path to current working directory.
    :param dest: destination where to save package.
    :return: package path
    """
    logger.debug(
    "Fetching {obj_type} {public_id} from Registry...".format(
    public_id=public_id, obj_type=obj_type
    )
    )
    logger.debug(
    "Downloading {obj_type} {public_id}...".format(
    public_id=public_id, obj_type=obj_type
    )
    )
    package_meta = get_package_meta(obj_type, public_id)
    file_url = cast(str, package_meta["file"])
    filepath = download_file(file_url, cwd)
    # next code line is needed because the items are stored in tarball packages as folders
    dest = os.path.split(dest)[0]
    logger.debug(
    "Extracting {obj_type} {public_id}...".format(
    public_id=public_id, obj_type=obj_type
    )
    )
    extract(filepath, dest)
    logger.debug(
    "Successfully fetched {obj_type} '{public_id}'.".format(
    public_id=public_id, obj_type=obj_type
    )
    )
    package_path = os.path.join(dest, public_id.name)
    return Path(package_path)

Choose a reason for hiding this comment

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

Please open an issue for this and add a comment in code.

Copy link
Author

Choose a reason for hiding this comment

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


def __lt__(self, other: Any) -> bool:
"""
Expand Down Expand Up @@ -647,12 +648,10 @@ def __repr__(self) -> str:
return f"PackageId{self.__str__()}"

def __eq__(self, other: Any) -> bool:
"""Compare with another object."""
return (
isinstance(other, PackageId)
and self.package_type == other.package_type
and self.public_id == other.public_id
)
"""Check equality."""
if not isinstance(other, self.__class__):
return NotImplemented # Delegate comparison to the other instance's __eq__.
return all(getattr(self, s) == getattr(other, s) for s in self.__slots__)

Comment on lines 623 to 628
Copy link
Author

Choose a reason for hiding this comment

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

__slots__ = ("_package_type", "_public_id")

def __lt__(self, other: Any) -> bool:
"""Compare two public ids."""
Expand Down Expand Up @@ -874,15 +873,10 @@ def __str__(self) -> str:
return f"{self.__class__.__name__}(name='{self.name}', version='{self.version}', index='{self.index}', git='{self.git}', ref='{self.ref}')"

def __eq__(self, other: Any) -> bool:
"""Compare with another object."""
return (
isinstance(other, Dependency)
and self._name == other._name
and self._version == other._version
and self._index == other._index
and self._git == other._git
and self._ref == other._ref
)
"""Check equality."""
if not isinstance(other, self.__class__):
return NotImplemented # Delegate comparison to the other instance's __eq__.
return all(getattr(self, s) == getattr(other, s) for s in self.__slots__)

Comment on lines 854 to 859
Copy link
Author

Choose a reason for hiding this comment

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

__slots__ = ("_name", "_version", "_index", "_git", "_ref")


Dependencies = Dict[str, Dependency]
Expand Down
6 changes: 3 additions & 3 deletions docs/api/configurations/data_types.md
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ Get the representation.
def __eq__(other: Any) -> bool
```

Compare with another object.
Check equality.

<a id="aea.configurations.data_types.PublicId.__lt__"></a>

Expand Down Expand Up @@ -795,7 +795,7 @@ Get the object representation in string.
def __eq__(other: Any) -> bool
```

Compare with another object.
Check equality.

<a id="aea.configurations.data_types.PackageId.__lt__"></a>

Expand Down Expand Up @@ -1080,7 +1080,7 @@ Get the string representation.
def __eq__(other: Any) -> bool
```

Compare with another object.
Check equality.

<a id="aea.configurations.data_types.Dependencies"></a>

Expand Down