Closed llucax closed 11 months ago
BTW, I was thinking of merging (no pun intended) both Merge
and MergeNamed
into the merge
module, but I didn't want to go for more changes in the RFC, we can do further fine-tuning if we agree to go with it.
So we discussed this and some people still preferred the more flat hierarchy, so we decided on the following:
Keep the _base_classes.py
split.
Rationale: Avoid circular dependencies.
Move all symbols to the top-level except from timer
and file_watcher
which are in their own module.
Rationale:
watchfiles
an optional dependency.file_watcher.Event
-> FileWatcherEvent
).Alternatives: Keep both in the util
or extensions
package. Discarded because it will pull the watchfiles
dependency even if you only want to use the Timer
class. Also thinking about Google-style importsusing from frequenz.channels import extensions
makes the module have a too generic name.
Although is not part of the restructuring itself, we also decided to:
Remove Bidirectional
and Peakable
Rationale: They were created as a hack for use cases that are not needed anymore. Pending check that Peakable
can really be removed in the SDK.
Future work:
Make timer
and file_watcher
separate python packages: frequenz-channels-timer
and frequenz-channels-file-watcher
. This way the pulling of dependencies is more explicit and also it is clear that they are not considered core components.
This might need support from repo-config
, as we need to see how we generate docs, etc. so we leave it out of 1.0.
Superseded by #235.
This is based in #231, so please only look at the last commit. Also it might be useful to just see the branch code instead of the diff: https://github.com/llucax/frequenz-channels-python/tree/structure-rfc/src/frequenz/channels
These are the most notable changes:
The
_base_classes
modules is split into the_receiver
and_sender
modules.Sender and receiver exceptions are moved from the
_exceptions
module to the new_sender
and_receiver
modules.The
frequenz.channel
package now only exposes the new_receiver
and_sender
modules, and the_exceptions
and_select
modules.All channels and receiver modules are moved to the
frequenz.channels
package and made public.All public nested classes were moved to the top level of the corresponding module.
Advantages of this structure:
It completely removes circular dependencies.
It avoids importing unnecessary code. In Python importing means code execution, so even when it is done only at startup, it adds some overhead.
Also by not importing unnecessary code, we can potentially add real optional dependencies. For example, if a project doesn't need to use a file watcher, they could avoid pulling the unnecessary
awatch
dependency. This is not done in this PR, but it could be done in the future.By having the channels and receivers on their own module we can move public nested classes were moved to the top level of the corresponding module withough having to add superflous prefixes for support classes.
Removing nested classes avoids having to use hacky constructions, like requiring the use of
from __future__ import annotations
, types as strings (nested classes) and confusing themkdocstrings
tools when extracting and cross-linking docs.The
frequenz.channels
package exposes all classes that are used once you have setted up your channels, so the importing should still be pretty terse in most cases and onlyfrequenz.channels
would need to be imported in modules only taking some receivers and iterating or selecting over them.Makes docs easier to navigate/discover, as the Table of contents becomes much smaller for each module and the module list is shown in the left.
Old:
vs. New: