Open PatrickBourdon opened 2 years ago
Thanks for the detailed analysis!
Just a quick answer to this part:
Why do you make private systematically every imported object, such as : import datetime as _datetime?
The purpose is to have clean namespaces: for example if we import datetime
in pypowsybl.network
, it will appear as being part of our own API, whereas it is not (you will never want to do from pypowsybl.network import datetime
, and you don't want your IDE to propose it with autocompletion, or it will be messy to use).
For now, we try to keep our namespaces tidy by importing other APIs with "private" scope. Maybe a more powerful solution, which seems broadly used, would be to move our implementations into inner modules, and build our public namespaces by importing in them only those pieces that we want to expose.
For example we could:
Network
in pypowsybl.network.core
with any import with any scopepypowsybl.network.__init__
: import Network
classThis should enable us to keep namespaces tidy, while not making imports private.
The work in #306 and #307 should solve some of the typing problems you reported:
_pypowsybl
mypy
static checker, that all methods have type annotations, and that there is no typing problem. Note though, that it might behave slightly differently from the pyright
static type check, which I think is included in pylance. In particular, for now, arguments are implicitly considered optional when their default value is None. This may be improved in a further evolution.Those improvements will be included in next release.
👍
Hi @PatrickBourdon
Did you have any opportunity to check if typing problems have indeed been solved in the last releases of pypowsybl, or if some remain ?
Fixed additional missing types for __members__
, following your mail feedback: #396
Do you want to request a feature or report a bug? Neither a bug nor a feature for a functional request, typing issues.
Please tell us about your environment:
typing: I use and recommend the use of python strict type checking, for it prevents many bugs and adds autodocumentation. I faced some typing issues using pypowsybl. To overcome them I created a local
ElementType(Enum)
class and apn.Network
subclass. Hereafter they are and the explaination follows:1/ Why
ElementType(Enum)
?pn.ElementType
orpn.ElementType.BUS
are both accessible respectively as<class '_pypowsybl.ElementType'>
and<ElementType.BUS: 0>
. However the definition of the latter is not accessible to the type checker and an error is reported when using any element type such as:df: DataFrame = network.get_elements(pn.ElementType.BUS)
2/ Why
pn.Network
subclass? To overloadget_elements
using theElementType(Enum)
without typing errors, whilst still using any other pypowsybl.network methods. The newget_elements
wraps (once for all) the typing errors, hidden by# type: ignore
.elements_count
is a basic example usingget_elements
: it reports the size of the loaded network.Note:
get_elements
is NOT described in the documentation but it is a public method.update_elements
also has apn.ElementType
parameter and it is part of the documentation.3/ About
__init__
: The method of the created subclass adds a feature: direct read of a.xiidm.gz
. The method wraps and hides the typing errors fromload_network_from_string
andload_network
.The typing errors come from their signatures:
parameters: dict = {}
.dict
alone misses some information. It should probably be something likedict[str, Any]
.Note that Using a
dict
as a default parameter is not recommended: but this is beyond typing. https://docs.quantifiedcode.com/python-anti-patterns/correctness/mutable_default_value_as_argument.html pylint notifies this as an error in strict mode. The recommended way is probablyparameters: Union[None, dict[str, Any]]=None
(python 3.9)parameters: None | dict[str, Any] = None
(python 3.10)4/ The file
network.py
includes the classpn.Network
and also reads:Use of
typint.List
(1) ettypint.Set
(3) are deprecated since 3.9: they are now justlist
andset
that nobody wants to make private (I am unsure if the new way to use them is available from python 3.7). https://docs.python.org/3/library/typing.html#typing.List https://docs.python.org/3/library/typing.html#typing.SetFrom the typing point-of-view, the types of parameters and result of any public method MUST be public. Otherwise the caller cannot type his variables, and a type checker in strict mode complains. A signature of a public method like
def my_method(self) -> _DataFrame:
is strange by essence, for it says that the result is a private type. However the good news is that the type checker understands:since imported
DataFrame == np._DataFrame
. Therefore (3) only prevents from writingfrom np import DataFrame
, which would be anyway harmless.Why do you make private systematically every imported object, such as :
import datetime as _datetime
?