refactor: get_poetry_deps leverages common.PoetryPackage
#18
Conversation
poetry_to_pre_commit/common.py
Outdated
| def __str__(self) -> str: | ||
| # Since the object is frozen, we can cache the result. But applying | ||
| # `lru_cache` on `__str__` makes trouble, so we use a cached property. | ||
| return self._str | ||
|
|
||
| def __lt__(self, value: object) -> bool: | ||
| return str(self) < str(value) | ||
|
|
||
| def __eq__(self, value: object) -> bool: | ||
| return str(self) == str(value) | ||
|
|
||
| def __hash__(self) -> int: | ||
| return hash(str(self)) | ||
|
|
||
| @cached_property | ||
| def _str(self) -> str: | ||
| if self.extras: | ||
| extras = ",".join(sorted(self.extras)) | ||
| return f"{self.name}[{extras}]=={self.version}" | ||
| else: | ||
| return f"{self.name}=={self.version}" |
There was a problem hiding this comment.
Hm? I'm not sure I understand why we need that ? dataclasses already provide this, no?
There was a problem hiding this comment.
__hash__ is needed because extras is unhashable.
__lt__ allows to sort a set of PoetryPackage.
And if you ovewrite __lt__, it makes sense to overwrite __eq__ too :-)
I added comments next to __lt__ and __hash__.
There was a problem hiding this comment.
Hm ?
>>> {frozenset(["a", "b"])}
{frozenset({'a', 'b'})}
>>> import dataclasses
>>> @dataclasses.dataclass(frozen=True)
... class X:
... extras: frozenset[str] = dataclasses.field(default_factory=frozenset)
...
>>> {X()}
{X(extras=frozenset())}Frozensets of strings are hashable, no ?
__lt__allows to sort a set ofPoetryPackage
You can usedataclass(order=True)for this
There was a problem hiding this comment.
Damn. The problem was that I didn't correctly build the objects in the tests, I was using set and not frozenset, and this mislead me. I removed __hash__.
There was a problem hiding this comment.
You can use dataclass(order=True) for this
Nice! No more dunder methods - beside __str__ - in common.PoetryPackage :-)
5405828 to
96c2947
Compare
Also add `extras` field to `common.PoetryPackage`. This allows to clean `update_or_remove_additional_deps` in `sync_hooks_additional_dependencies.py`
96c2947 to
979bc38
Compare
Also add
extrasfield tocommon.PoetryPackage. This allows to cleanupdate_or_remove_additional_depsinsync_hooks_additional_dependencies.pySuccessful PR Checklist:
PR label(s):