-
Notifications
You must be signed in to change notification settings - Fork 362
memory2 StreamModules, cleanup #1682
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
base: dev
Are you sure you want to change the base?
Changes from all commits
7a23d45
fdf9bd4
b0ad0bc
607ee10
3d7becc
4ffc0ff
c936444
329f263
423128f
da56191
2057960
bac18cc
fb0b970
9ca9019
a71f914
ac838a2
5e91c8c
b755dee
39206ab
8f5577b
4dd60c0
fb962be
ae6de37
05fd764
8e1e7f1
d04ade4
338713e
70980ff
7a538d3
840f874
71fc3a9
3a19b4e
abb2fbf
331d821
b84f96c
b5c2ae8
a3ff4ab
207e763
b56517d
503e992
49c9b64
51745e9
1b5f3d4
f135766
889d153
9aabcd3
5fe848f
07b6609
469da56
88aa58a
b0f04c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ | |
|
|
||
| from abc import abstractmethod | ||
| import sys | ||
| from typing import TYPE_CHECKING | ||
| from typing import TYPE_CHECKING, TypeVar | ||
|
|
||
| if sys.version_info >= (3, 11): | ||
| from typing import Self | ||
|
|
@@ -29,6 +29,8 @@ | |
| from reactivex.abc import DisposableBase | ||
| from reactivex.disposable import CompositeDisposable | ||
|
|
||
| D = TypeVar("D", bound=DisposableBase) | ||
|
|
||
|
|
||
| class Resource(DisposableBase): | ||
| @abstractmethod | ||
|
|
@@ -75,18 +77,17 @@ def __exit__( | |
| class CompositeResource(Resource): | ||
| """Resource that owns child disposables, disposed on stop().""" | ||
|
|
||
| _disposables: CompositeDisposable | ||
|
|
||
| def __init__(self) -> None: | ||
| self._disposables = CompositeDisposable() | ||
| _disposables: CompositeDisposable | None = None | ||
|
|
||
| def register_disposables(self, *disposables: DisposableBase) -> None: | ||
| """Register child disposables to be disposed when this resource stops.""" | ||
| for d in disposables: | ||
| self._disposables.add(d) | ||
| def register_disposable(self, disposable: D) -> D: | ||
| """Register a child disposable to be disposed when this resource stops.""" | ||
| if self._disposables is None: | ||
| self._disposables = CompositeDisposable() | ||
| self._disposables.add(disposable) | ||
| return disposable | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this return the disposable?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. convinience API, you often create an object, and want to register as disposable bla = self.register_disposable(new Bla())
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| def start(self) -> None: | ||
| pass | ||
| def start(self) -> None: ... | ||
|
|
||
| def stop(self) -> None: | ||
| self._disposables.dispose() | ||
| if self._disposables is not None: | ||
| self._disposables.dispose() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous version was better.
self._disposablesshould be set in__init__.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a bunch of classes to this (all modules) so I don't want to create overhead of creating a compositedisposable if not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I actually didn't realize this before, but ModuleBase already has
self._disposables. This is why multiple inheritance is confusing. disposables are created in ModuleBase, but also in CompositeResource.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aha ok, can I remove from modules? on-demand CompositeResource is so convinient for Modules