pypowsybl icon indicating copy to clipboard operation
pypowsybl copied to clipboard

Public namespaces and python code layout

Open sylvlecl opened this issue 3 years ago • 2 comments

  • Do you want to request a feature or report a bug?

Refactoring

  • What is the current behavior?

Public namespace management is done mainly by importing as "private" the things we don't want to expose in our public API. It makes the code uneasy to read and can apparently cause some type resolution issues with some IDEs (#298).

Anyway, the current layout will not allow the lib to scale very well, since all the code is packed in a few public files (pypowsybl/network.py ...).

  • What is the expected behavior?

In order to have clean public namespaces while allowing to scale better, I propose one of the 2 following layouts.

First one is more "distributed" and second one more centralized with most implementation in "core" package.

pypowsybl/
  |    __init__.py
  |
  +-- network/
  |    __init__.py  # Public objects are imported here
  |    network.py  # Implementation of Network goes there
  |    networks.py  # As an example: methods for "pre-built" networks go there
  |    ...
  +-- loadflow/
  |    __init__.py   # Public objects are imported here
  |    ac.py
  |    dc.py
  |    validation.py
  |    ...
  +-- security/
       ...

The other possible layout, more centralized in "core" package:

pypowsybl/
  |    __init__.py
  |
  +-- core/  # Most of the implementation goes there
  |   |    __init__.py
  |   +-- network/
  |   |    __init__.py # intermediate level of aggregation
  |   |   network.py
  |   + ...
  |
  +-- network/
  |    __init__.py  # from core.network import Network ...
  +-- loadflow/
  |    __init__.py   # from core.loadflow import run_ac, run_dc
  +-- security/
       ...

sylvlecl avatar Jan 31 '22 10:01 sylvlecl

can apparently cause some type resolution issues with some IDEs

I think you miss the point that any code refactoring won't fix, as soon as "strict type checking" is on. The reason is the type checker requires some missing elements: let's be more precise.

The following code runs without error:

>>> import pypowsybl.network as pn
>>> network = pn.create_empty()
>>> generators = network.get_elements(pn.ElementType.GENERATOR)
>>> print(generators)
Empty DataFrame
Columns: [name, energy_source, target_p, min_p, max_p, min_q, max_q, target_v, target_q, voltage_regulator_on, p, q, i, voltage_level_id, bus_id, connected]
Index: []

To check that the get_elements method is called with correctly typed parameters, the type checker uses the method signature:

def get_elements(self, element_type: _pypowsybl.ElementType) -> _DataFrame:

Inspection at run time shows that _pypowsybl.ElementType is a class, some sort of Enum subclass, and that pn.ElementType.GENERATOR is one of its members.

>>> type(pn.ElementType)
<class 'pybind11_builtins.pybind11_type'>
>>> pn.ElementType.GENERATOR in pn.ElementType.__members__.values()
True

However when only inspecting the source code - as the type checker does - _pypowsybl.ElementType is imported from some inaccessible code (so it could be any type of object) and pn.ElementType.GENERATOR is nowhere defined and can be any property or method with unknown type..

Hence, whatever IDE and whatever type checker, as soon "strict type checking" is activated, my guess is that every use of a pn.ElementType member will produce a type checker error report, without any possible workaround other than # type: ignore.

Maybe the solution is to create some pypowsybl-stubs, but this is out of my confort zone.

PatrickBourdon avatar Feb 01 '22 23:02 PatrickBourdon

Yes, I'm aware this issue will not solve all the issues you described in #298 , in particular it will not provide type information for the underlying C API. It will only answer the part about "why private types appear in the signatures". Indeed for the C API part, we need to look for the possible technical solutions too, and we'll open another issue for that.

The refactoring described in this issues is something we must do to be able to scale the library correctly in the future, independently of typing and scopes matters.

sylvlecl avatar Feb 02 '22 09:02 sylvlecl