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

On Mon, Jul 27, 2020 at 08:32:29AM +0200, Philipp Hahn wrote: > 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] I would argue that ^this was never the intended way of using the returned object and would lean towards using a named tuple, since like you've pointed out it would be a transparent change in the vast majority of cases. However, since we don't document anywhere how the return value should be treated, from that perspective it's still valid to change the returned list for whatever purposes and therefore we are breaking the API contract, which we can't do even though the change itself is reasonable.