Skip to content

Mock resampler: Improve variable names #879

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 14, 2024
Merged

Mock resampler: Improve variable names #879

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 16 additions & 13 deletions tests/timeseries/mock_resampler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def __init__( # pylint: disable=too-many-arguments
self._resampler_request_channel = Broadcast[ComponentMetricRequest](
"resampler-request"
)
self._basic_receivers: dict[str, list[Receiver[Sample[Quantity]]]] = {}
self._input_channels_receivers: dict[str, list[Receiver[Sample[Quantity]]]] = {}

def power_senders(
comp_ids: list[int],
Expand All @@ -58,7 +58,7 @@ def power_senders(
Sample[Quantity], name
).new_sender()
)
self._basic_receivers[name] = [
self._input_channels_receivers[name] = [
self._channel_registry.get_or_create(
Sample[Quantity], name
).new_receiver()
Expand All @@ -77,7 +77,7 @@ def frequency_senders(
Sample[Quantity], name
).new_sender()
)
self._basic_receivers[name] = [
self._input_channels_receivers[name] = [
self._channel_registry.get_or_create(
Sample[Quantity], name
).new_receiver(name)
Expand Down Expand Up @@ -119,19 +119,19 @@ def multi_phase_senders(
).new_sender(),
]
)
self._basic_receivers[p1_name] = [
self._input_channels_receivers[p1_name] = [
self._channel_registry.get_or_create(
Sample[Quantity], p1_name
).new_receiver()
for _ in range(namespaces)
]
self._basic_receivers[p2_name] = [
self._input_channels_receivers[p2_name] = [
self._channel_registry.get_or_create(
Sample[Quantity], p2_name
).new_receiver()
for _ in range(namespaces)
]
self._basic_receivers[p3_name] = [
self._input_channels_receivers[p3_name] = [
self._channel_registry.get_or_create(
Sample[Quantity], p3_name
).new_receiver()
Expand Down Expand Up @@ -222,15 +222,18 @@ async def _handle_resampling_requests(self) -> None:
name = request.get_channel_name()
if name in self._forward_tasks:
continue
basic_recv_name = f"{request.component_id}:{request.metric_id}"
recv = self._basic_receivers[basic_recv_name].pop()
assert recv is not None
input_chan_recv_name = f"{request.component_id}:{request.metric_id}"
input_chan_recv = self._input_channels_receivers[input_chan_recv_name].pop()
assert input_chan_recv is not None
output_chan_sender: Sender[Sample[Quantity]] = (
self._channel_registry.get_or_create(
Sample[Quantity], name
).new_sender()
)
Comment on lines +225 to +232
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, this is better than basic_receivers, but input_receivers and output_sender might have even nicer. input_channels_receivers make me think that they receive channels and not input values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, at first I named it without the chan, but for some reason it still seemed ambiguous, but now I don't remember why. Maybe @daniel-zullo-frequenz as he was helping me to understand the code while I did it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC we wanted to mainly get rid of basic in the variable name because it was confusing/difficult-to-understand what a basic_receiver was. I do agree having only receivers and sender in the name would have suffice to understand they are channels given the context. However I don't think it causes any misunderstanding if we keep chan/channels explicitly in the variable names. I mean I haven't seen yet any scenarios where we send/receive channels

task = asyncio.create_task(
self._channel_forward_messages(
recv,
self._channel_registry.get_or_create(
Sample[Quantity], name
).new_sender(),
input_chan_recv,
output_chan_sender,
),
name=name,
)
Expand Down