For now this is only used by sorted_sha1, but we'll need it for
origin_metadata soon.
Details
- Reviewers
ardumont - Group Reviewers
Reviewers - Commits
- rDSTOfe5600555512: Add a wrapper to manage a sorted list.
Diff Detail
- Repository
- rDSTO Storage manager
- Branch
- bisect-wrapper
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 11989 Build 18179: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 18178: arc lint + arc unit
Event Timeline
Build is green
Patch application report for D2987 (id=10625)
Rebasing onto b0b0313c96...
Current branch diff-target is up to date.
Changes applied before test
commit 5da27ab73280c29ab6355e04cd4d67fc44ff365c Author: Valentin Lorentz <vlorentz@softwareheritage.org> Date: Tue Mar 24 17:22:09 2020 +0100 Add a wrapper to manage a sorted list. For now this is only used by sorted_sha1, but we'll need it for origin_metadata soon.
See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/78/ for more details.
I am not sure if explicitly setting the key type when instantiating a SortedList instance is a real necessity.
How about simplifying a little bit in order to simply write sha1s = SortedList[bytes] ?
SortedListItem = TypeVar("SortedListItem") SortedListKey = Any class SortedList(collections.UserList, Generic[SortedListItem]): data: List[Tuple[SortedListKey, SortedListItem]] # https://github.com/python/mypy/issues/708 # key: Callable[[SortedListItem], SortedListKey] def __init__( self, data: List[SortedListItem] = None, key: Optional[Callable[[SortedListItem], SortedListKey]] = None, ): super().__init__(sorted(data or [], key=key)) self.key: Optional[Callable[[SortedListItem], SortedListKey]] = key def add(self, item: SortedListItem): k = self.key(item) if self.key is not None else item bisect.insort(self.data, (k, item)) def __iter__(self) -> Iterator[SortedListItem]: for (k, item) in self.data: yield item def iter_from(self, start_key: SortedListKey) -> Iterator[SortedListItem]: from_index = bisect.bisect_left(self.data, (start_key,)) for (k, item) in itertools.islice(self.data, from_index, None): yield item
Also some simple tests are missing.
swh/storage/in_memory.py | ||
---|---|---|
129 | You can also write: self._shorted_sha1s = SortedList[bytes, bytes]() |
swh/storage/in_memory.py | ||
---|---|---|
129 | TIL, thanks |
Of course it's not necessary, but this way mypy can check the type of key and argument of iter_from match.
Build is green
Patch application report for D2987 (id=10772)
Rebasing onto 32b3e93695...
Current branch diff-target is up to date.
Changes applied before test
commit 4526c528ed9d5d5788fac4c4f25ab3e468521a49 Author: Valentin Lorentz <vlorentz@softwareheritage.org> Date: Tue Mar 24 17:22:09 2020 +0100 Add a wrapper to manage a sorted list. For now this is only used by sorted_sha1, but we'll need it for origin_metadata soon.
See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/100/ for more details.
swh/storage/in_memory.py | ||
---|---|---|
64 | why not Tuple[SortedListKey, ...]? |
swh/storage/in_memory.py | ||
---|---|---|
64 | i'll be explicit to avoid confusion, i meant why not List[Tuple[SortedListKey, SortedListItem]]? |
swh/storage/tests/test_in_memory.py | ||
---|---|---|
61 | Arf... Could you please make that more apparent? For example: assert list(list_.iter_from(split)) == expected, f"split: {split}" or something better if you have ;) |
Thanks.
I'm fine with the implementation, just a nitpick on the assertions which i misunderstood initially.
swh/storage/in_memory.py | ||
---|---|---|
64 | oh yes, indeed |
Build is green
Patch application report for D2987 (id=10834)
Rebasing onto bca643acab...
First, rewinding head to replay your work on top of it... Applying: Add a wrapper to manage a sorted list.
Changes applied before test
commit 0a31410ed5e22c77113c35c4f2fd24b30d9c2d87 Author: Valentin Lorentz <vlorentz@softwareheritage.org> Date: Tue Mar 24 17:22:09 2020 +0100 Add a wrapper to manage a sorted list. For now this is only used by sorted_sha1, but we'll need it for origin_metadata soon.
See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/112/ for more details.
Build is green
Patch application report for D2987 (id=10844)
Rebasing onto fe56005555...
First, rewinding head to replay your work on top of it... Fast-forwarded diff-target to base-revision-114-D2987.
Changes applied before test
See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/114/ for more details.