Skip to content

Commit

Permalink
refactor: consider order in hash methods and make _markers attribute …
Browse files Browse the repository at this point in the history
…a tuple instead of a list because it must not be changed
  • Loading branch information
radoering committed Aug 5, 2023
1 parent f955082 commit 9274aae
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 40 deletions.
6 changes: 1 addition & 5 deletions src/poetry/core/constraints/generic/multi_constraint.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,7 @@ def __eq__(self, other: object) -> bool:
return self._constraints == other._constraints

def __hash__(self) -> int:
h = hash("multi")
for constraint in self._constraints:
h ^= hash(constraint)

return h
return hash(("multi", *self._constraints))

def __str__(self) -> str:
constraints = [str(constraint) for constraint in self._constraints]
Expand Down
6 changes: 1 addition & 5 deletions src/poetry/core/constraints/generic/union_constraint.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,7 @@ def __eq__(self, other: object) -> bool:
return self._constraints == other._constraints

def __hash__(self) -> int:
h = hash("union")
for constraint in self._constraints:
h ^= hash(constraint)

return h
return hash(("union", *self._constraints))

def __str__(self) -> str:
constraints = [str(constraint) for constraint in self._constraints]
Expand Down
46 changes: 20 additions & 26 deletions src/poetry/core/version/markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def __repr__(self) -> str:
return "<AnyMarker>"

def __hash__(self) -> int:
return hash(("<any>", "<any>"))
return hash("any")

def __eq__(self, other: object) -> bool:
if not isinstance(other, BaseMarker):
Expand Down Expand Up @@ -193,7 +193,7 @@ def __repr__(self) -> str:
return "<EmptyMarker>"

def __hash__(self) -> int:
return hash(("<empty>", "<empty>"))
return hash("empty")

def __eq__(self, other: object) -> bool:
if not isinstance(other, BaseMarker):
Expand Down Expand Up @@ -232,6 +232,10 @@ def name(self) -> str:
def constraint(self) -> SingleMarkerConstraint:
return self._constraint

@property
def _key(self) -> tuple[object, ...]:
return self._name, self._constraint

def validate(self, environment: dict[str, Any] | None) -> bool:
if environment is None:
return True
Expand Down Expand Up @@ -284,10 +288,10 @@ def __eq__(self, other: object) -> bool:
if not isinstance(other, SingleMarkerLike):
return NotImplemented

return self._name == other.name and self._constraint == other.constraint
return self._key == other._key

def __hash__(self) -> int:
return hash((self._name, self._constraint))
return hash(self._key)


class SingleMarker(SingleMarkerLike[Union[BaseConstraint, VersionConstraint]]):
Expand Down Expand Up @@ -368,6 +372,10 @@ def operator(self) -> str:
def value(self) -> str:
return self._value

@property
def _key(self) -> tuple[object, ...]:
return self._name, self._operator, self._value

def invert(self) -> BaseMarker:
if self._operator in ("===", "=="):
operator = "!="
Expand Down Expand Up @@ -417,10 +425,10 @@ def __eq__(self, other: object) -> bool:
if not isinstance(other, SingleMarker):
return NotImplemented

return str(self) == str(other)
return self._key == other._key

def __hash__(self) -> int:
return hash(str(self))
return hash(self._key)

def __str__(self) -> str:
return f'{self._name} {self._operator} "{self._value}"'
Expand Down Expand Up @@ -503,10 +511,10 @@ def _flatten_markers(

class MultiMarker(BaseMarker):
def __init__(self, *markers: BaseMarker) -> None:
self._markers = _flatten_markers(markers, MultiMarker)
self._markers = tuple(_flatten_markers(markers, MultiMarker))

@property
def markers(self) -> list[BaseMarker]:
def markers(self) -> tuple[BaseMarker, ...]:
return self._markers

@property
Expand Down Expand Up @@ -657,11 +665,7 @@ def __eq__(self, other: object) -> bool:
return self._markers == other.markers

def __hash__(self) -> int:
h = hash("multi")
for m in self._markers:
h ^= hash(m)

return h
return hash(("multi", *self._markers))

def __str__(self) -> str:
elements = []
Expand All @@ -676,10 +680,10 @@ def __str__(self) -> str:

class MarkerUnion(BaseMarker):
def __init__(self, *markers: BaseMarker) -> None:
self._markers = _flatten_markers(markers, MarkerUnion)
self._markers = tuple(_flatten_markers(markers, MarkerUnion))

@property
def markers(self) -> list[BaseMarker]:
def markers(self) -> tuple[BaseMarker, ...]:
return self._markers

@property
Expand Down Expand Up @@ -744,12 +748,6 @@ def of(cls, *markers: BaseMarker) -> BaseMarker:

return MarkerUnion(*new_markers)

def append(self, marker: BaseMarker) -> None:
if marker in self._markers:
return

self._markers.append(marker)

def intersect(self, other: BaseMarker) -> BaseMarker:
return intersection(self, other)

Expand Down Expand Up @@ -837,11 +835,7 @@ def __eq__(self, other: object) -> bool:
return self._markers == other.markers

def __hash__(self) -> int:
h = hash("union")
for m in self._markers:
h ^= hash(m)

return h
return hash(("union", *self._markers))

def __str__(self) -> str:
return " or ".join(str(m) for m in self._markers)
Expand Down
8 changes: 4 additions & 4 deletions tests/version/test_markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,10 @@ def test_multi_marker() -> None:
m = parse_marker('sys_platform == "darwin" and implementation_name == "cpython"')

assert isinstance(m, MultiMarker)
assert m.markers == [
assert m.markers == (
parse_marker('sys_platform == "darwin"'),
parse_marker('implementation_name == "cpython"'),
]
)


def test_multi_marker_is_empty_is_contradictory() -> None:
Expand Down Expand Up @@ -652,10 +652,10 @@ def test_marker_union() -> None:
m = parse_marker('sys_platform == "darwin" or implementation_name == "cpython"')

assert isinstance(m, MarkerUnion)
assert m.markers == [
assert m.markers == (
parse_marker('sys_platform == "darwin"'),
parse_marker('implementation_name == "cpython"'),
]
)


def test_marker_union_deduplicate() -> None:
Expand Down

0 comments on commit 9274aae

Please sign in to comment.