Resolve potential cyclic import issues

[leandro-lucarella-frequenz]

To have a clean public interface we are defining some top level sub-packages that expose classes in some sub-packages or sub-modules, but some of these top-level packages have or can have cyclic dependencies. For example, with the introduction of the LogicalMeter, which uses the DataSourcingActor, we have the actor package depending on the timeseries packages and viceversa (currently we are not exposing the LogicalMeter directly in timeseries because of this.

There are 2 main paths that I can see here:

1. Avoid cyclic dependencies between packages.
2. Don't expose sub-modules symbols in packages (and use them instead as namespaces for other modules).

I noticed some other issues with approach 1., like in actor, it doesn't look right to expose classes supporting actor implementations (like Request / Response in PowerDistirbutingActor or ComponentMetricRequest in DataSourcingActor) and it makes sense to have a namespace for these (this is actually why the PowerDistributingActor ended up with its own sub-module).

This also can allow for shorter class names, for example: actor.power_distributing.Actor, which also plays nicely with Google coding standard (which we are supposed to be using) if we use from frequenz.sdk.actor import power_distributing; pd = power_distributing.Actor(...); pd_sender.send(power_distributing.Request(....)).

It is a bit of a drastic change, but it might make sense.

Adopting 1. might look as a less drastic change, but in reality it introduces artificial limitations in how we want to structure our API.

_Coming from @leandro-lucarella-frequenz in https://github.com/frequenz-floss/frequenz-sdk-python/pull/66#discussion_r1032685146_

[leandro-lucarella-frequenz]

Related to:

• 83 (which follows option 1)

[leandro-lucarella-frequenz]

I'm more and more convinced that option 2 is the way to go, as I think we'll keep bumping into these issues again and again if we go with option 1. Also as the SDK grows, I think approach 1 will end up with packages with too many symbols in them, making it even more likely for cyclic dependencies to happen. This goes beyond the discussion about if classes used to communicate with actors should live in the actor module or not.

I know this would mean another change in the package structure, and I take the blame on it, I think I did the previous round too rushed to have the cleanup before the release without thinking it more thoroughly.

For example, even when importing .timeseries._sample internally, this needs to first initialize the .timeseries package, which runs timeseries/__init__.py, which in turn do a bunch of other import, including .timeseries._logical_meter, which in turn runs timeseries/_logical_meter/__init__.py, which will import all modules in timeseries/_logical_meter/*.py. So it is easy to see how things get out of hand pretty soon.

If we don't import sub-modules in packages, but instead we allow importing the parent package from a sub-module, then things are fine (so we can use parent packages to define common stuff only, but not to re-export what is defined in sub-modules):

So if we make timeseries/__init__.py only import timeseries/_sample.py, then we can do from frequenz.sdk.timeseries import Sample (this will only run timeseries/__init__.py and timeseries/_sample.py but nothing else), and in, for example, and inside of timeseries/logical_meter/ we can from .. import Sample too without the risk of running into indirect cyclic dependencies. from frequenz.sdk.timeseries.logical_meter import LogicalMeter run timeseries/__init__.py but that will be it.

I think to be able to fully prove this we need to eventually do a PR to change the structure and see how it works and feels.

[leandro-lucarella-frequenz]

Summary from last meeting:

• The timseries package should only have classes that operate on purely data streams (Receivers).
• The classes that depend on microgrid components (like LogicalMeter, BatteryPool, etc.) should go to the microgrid package. For now we said to leave it in the top-level, as we didn't find a good name for a grouping package (I suggested aggregation as it seems to be a common characteristic of LogicalMeter, etc., but it is a bit of stretch).
• We are not going to rush a new restructuring to avoid making users life miserable by changing the structure on every release, so we'll watch out if this approach still makes sense for a while, probably adding new classes in the new places but not moving old ones yet.

[llucax]

Blocked on:

• 575

[llucax]

Actually closing, we'll just make sure we avoid cyclic imports in #575.