improve compare-machine-types.py

[PATCH 2/2] scripts/compare-mt: stop using global variables

Posted by Maksim Davydov 3 days, 18 hours ago

All variables inside the main if-structure are global that can be
confusing or be the reason of an issue. So, all code inside this structure
was moved to the separate function to detect all usages of these global
variables. All these usages were deleted.

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
---
 scripts/compare-machine-types.py | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/scripts/compare-machine-types.py b/scripts/compare-machine-types.py
index dba9e03548..e6fcea22e1 100755
--- a/scripts/compare-machine-types.py
+++ b/scripts/compare-machine-types.py
@@ -264,7 +264,6 @@ def __init__(self, vm: QEMUMachine,
                  req_mt: List[str], all_mt: bool) -> None:
         self._vm = vm
         self._binary = vm.binary
-        self._qemu_args = args.qemu_args.split(' ')
 
         self._qemu_drivers = VMPropertyGetter(vm)
         self.req_mt = get_req_mt(self._qemu_drivers, vm, req_mt, all_mt)
@@ -482,17 +481,17 @@ def fill_prop_table(configs: List[Configuration],
 
 def print_table(table: pd.DataFrame, table_format: str) -> None:
     if table_format == 'json':
-        print(comp_table.to_json())
+        print(table.to_json())
     elif table_format == 'csv':
-        print(comp_table.to_csv())
+        print(table.to_csv())
     else:
-        print(comp_table.to_markdown(index=False, stralign='center',
-                                     colalign=('center',), headers='keys',
-                                     tablefmt='fancy_grid',
-                                     disable_numparse=True))
+        print(table.to_markdown(index=False, stralign='center',
+                                colalign=('center',), headers='keys',
+                                tablefmt='fancy_grid',
+                                disable_numparse=True))
 
 
-if __name__ == '__main__':
+def main() -> None:
     args = parse_args()
     with ExitStack() as stack:
         vms = [stack.enter_context(QEMUMachine(binary=binary, qmp_timer=15,
@@ -506,3 +505,7 @@ def print_table(table: pd.DataFrame, table_format: str) -> None:
         comp_table = fill_prop_table(configurations, args.raw)
         if not comp_table.empty:
             print_table(comp_table, args.format)
+
+
+if __name__ == '__main__':
+    main()
-- 
2.34.1

Re: [PATCH 2/2] scripts/compare-mt: stop using global variables

Posted by Vladimir Sementsov-Ogievskiy 3 days, 16 hours ago

On 23.10.25 19:58, Maksim Davydov wrote:
> All variables inside the main if-structure are global that can be
> confusing or be the reason of an issue. So, all code inside this structure
> was moved to the separate function to detect all usages of these global
> variables. All these usages were deleted.
> 

May be mention, that Configuration._qemu_args was unused.

> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

> ---
>   scripts/compare-machine-types.py | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/scripts/compare-machine-types.py b/scripts/compare-machine-types.py
> index dba9e03548..e6fcea22e1 100755
> --- a/scripts/compare-machine-types.py
> +++ b/scripts/compare-machine-types.py
> @@ -264,7 +264,6 @@ def __init__(self, vm: QEMUMachine,
>                    req_mt: List[str], all_mt: bool) -> None:
>           self._vm = vm
>           self._binary = vm.binary
> -        self._qemu_args = args.qemu_args.split(' ')
>   
>           self._qemu_drivers = VMPropertyGetter(vm)
>           self.req_mt = get_req_mt(self._qemu_drivers, vm, req_mt, all_mt)
> @@ -482,17 +481,17 @@ def fill_prop_table(configs: List[Configuration],
>   
>   def print_table(table: pd.DataFrame, table_format: str) -> None:
>       if table_format == 'json':
> -        print(comp_table.to_json())
> +        print(table.to_json())
>       elif table_format == 'csv':
> -        print(comp_table.to_csv())
> +        print(table.to_csv())
>       else:
> -        print(comp_table.to_markdown(index=False, stralign='center',
> -                                     colalign=('center',), headers='keys',
> -                                     tablefmt='fancy_grid',
> -                                     disable_numparse=True))
> +        print(table.to_markdown(index=False, stralign='center',
> +                                colalign=('center',), headers='keys',
> +                                tablefmt='fancy_grid',
> +                                disable_numparse=True))
>   
>   
> -if __name__ == '__main__':
> +def main() -> None:
>       args = parse_args()
>       with ExitStack() as stack:
>           vms = [stack.enter_context(QEMUMachine(binary=binary, qmp_timer=15,
> @@ -506,3 +505,7 @@ def print_table(table: pd.DataFrame, table_format: str) -> None:
>           comp_table = fill_prop_table(configurations, args.raw)
>           if not comp_table.empty:
>               print_table(comp_table, args.format)
> +
> +
> +if __name__ == '__main__':
> +    main()


-- 
Best regards,
Vladimir

Re: [PATCH 2/2] scripts/compare-mt: stop using global variables

Posted by Maksim Davydov 3 days, 2 hours ago

Thanks for reviewing!

On 10/23/25 21:45, Vladimir Sementsov-Ogievskiy wrote:
> On 23.10.25 19:58, Maksim Davydov wrote:
>> All variables inside the main if-structure are global that can be
>> confusing or be the reason of an issue. So, all code inside this 
>> structure
>> was moved to the separate function to detect all usages of these global
>> variables. All these usages were deleted.
>>
> 
> May be mention, that Configuration._qemu_args was unused.

Yes, I'll add a comment about _qemu_args to the commit message.

> 
>> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> 
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> 
>> ---
>>   scripts/compare-machine-types.py | 19 +++++++++++--------
>>   1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/scripts/compare-machine-types.py b/scripts/compare- 
>> machine-types.py
>> index dba9e03548..e6fcea22e1 100755
>> --- a/scripts/compare-machine-types.py
>> +++ b/scripts/compare-machine-types.py
>> @@ -264,7 +264,6 @@ def __init__(self, vm: QEMUMachine,
>>                    req_mt: List[str], all_mt: bool) -> None:
>>           self._vm = vm
>>           self._binary = vm.binary
>> -        self._qemu_args = args.qemu_args.split(' ')
>>           self._qemu_drivers = VMPropertyGetter(vm)
>>           self.req_mt = get_req_mt(self._qemu_drivers, vm, req_mt, 
>> all_mt)
>> @@ -482,17 +481,17 @@ def fill_prop_table(configs: List[Configuration],
>>   def print_table(table: pd.DataFrame, table_format: str) -> None:
>>       if table_format == 'json':
>> -        print(comp_table.to_json())
>> +        print(table.to_json())
>>       elif table_format == 'csv':
>> -        print(comp_table.to_csv())
>> +        print(table.to_csv())
>>       else:
>> -        print(comp_table.to_markdown(index=False, stralign='center',
>> -                                     colalign=('center',), 
>> headers='keys',
>> -                                     tablefmt='fancy_grid',
>> -                                     disable_numparse=True))
>> +        print(table.to_markdown(index=False, stralign='center',
>> +                                colalign=('center',), headers='keys',
>> +                                tablefmt='fancy_grid',
>> +                                disable_numparse=True))
>> -if __name__ == '__main__':
>> +def main() -> None:
>>       args = parse_args()
>>       with ExitStack() as stack:
>>           vms = [stack.enter_context(QEMUMachine(binary=binary, 
>> qmp_timer=15,
>> @@ -506,3 +505,7 @@ def print_table(table: pd.DataFrame, table_format: 
>> str) -> None:
>>           comp_table = fill_prop_table(configurations, args.raw)
>>           if not comp_table.empty:
>>               print_table(comp_table, args.format)
>> +
>> +
>> +if __name__ == '__main__':
>> +    main()
> 
> 

-- 
Best regards,
Maksim Davydov