libvirt-python: API change List → (Named)Tuple?

[Philipp Hahn]

Hello,

Am 25.07.20 um 23:45 schrieb Philipp Hahn:
> Am 27.04.20 um 15:44 schrieb Philipp Hahn:
>> I'm working on adding PEP 484 type hints
>> <https://www.python.org/dev/peps/pep-0484/> to the Python binding of
>> libvirt.
...
> I just opened a merge request
> <https://gitlab.com/libvirt/libvirt-python/-/merge_requests/9> for my
> code at <https://gitlab.com/pmhahn/libvirt-python/-/tree/typing>.

While working on that I stumbled over some annoyances in the current
Python API: There are many methods which return a List[int], for example:
  virDomainGetInfo
  virDomainGetState
  virDomainGetControlInfo
  virDomainGetBlockInfo
  virDomainGetJobInfo
  virStoragePoolGetInfo
  virStorageVolGetInfo
  virStorageVolGetInfoFlags

There are more function returning similar information as plain List:
  virNodeGetSecurityModel
  virNodeGetSecurityModel
  virDomainGetSecurityLabelList
  virDomainBlockStats
  virDomainInterfaceStats
  virDomainGetVcpus
  virNodeGetCPUMap

The worst offender is `virNodeGetInfo`, which returns `Tuple[str,
int*7]`: The problem for type annotation is that `List` is unlimited in
the number of elements, that is you cannot specify the number of entries
the list must or will contain.
Also all elements of a list must have the same (super-)type; for "str"
and "int" of "virNodeGetInfo()" that would be "Any", which is not very
useful here.

A better type for those `List`s would be `Tuple`, which has a fixed
length and can have different types for each position.

But that would be an API change: In most cases users of those functions
should not notice the difference as indexing tuples or lists is the same.
It would break code where someone would do something like this:
  ret = virFunction_returning_list()
  ret += [1, 2, 3]
which breaks if that function would return a `Tuple` in the future:

> $ python -c '() + []'
> Traceback (most recent call last):
>   File "<string>", line 1, in <module>
> TypeError: can only concatenate tuple (not "list") to tuple

Even better then plain `Tuple`s would be
<https://docs.python.org/3/library/collections.html#collections.namedtuple>,
which would allow us to use the return value of `virNodeGetInfo()` as this:

> from collections import namedtuple
> import  libvirt
> virNodeInfo = namedtuple("virNodeInfo", ["model", "memory", "cpus", "mhz", "nodes", "sockets", "cores", "threads"])
> c = libvirt.open('test:///default')
> info = virNodeInfo(*c.getInfo())
> print(info.model)
> print(info)
> # virNodeInfo(model='i686', memory=3072, cpus=16, mhz=1400, nodes=2, sockets=2, cores=2, threads=2)

<https://libvirt.org/html/libvirt-libvirt-host.html#virNodeInfo>

This could be improved even more by using
<https://mypy.readthedocs.io/en/stable/kinds_of_types.html?highlight=NamedTuple#named-tuples>,
which allows to add type information:

> from typing import NamedTuple
> virNodeInfo = NamedTuple("virNodeInfo", [("model", str), ("memory", int), ("cpus", int), ("mhz", int), ("nodes", int), ("sockets", int), ("cores", int), ("threads", int)])

or with Python 3.6 the more readable form

> class virNodeInfo(NamedTuple):
>   model: str
>   memory: int
>   cpus: int
>   mhz: int
>   nodes: int
>   sockets: int
>   cores: int
>   threads: int

IMHO that would be a real usability improvement as I have to lookup that
information every time I have to use those functions myself.
Indexing with `namedtuple` and `NamedTuple` still works, so you can
still use `info[0]` above to get the model.

What do you think of that?
When would be a good time to make such a change?

Philipp