Re: [PATCH 3/3] rteval: systopology: Slight CpuList.__expand_cpulist() cleanup

[Valentin Schneider <vschneid@redhat.com>]

On 29/04/22 16:53, John Kacur wrote:
> On Tue, 19 Apr 2022, Valentin Schneider wrote:
>
>> This method currently aggregates CPUs into a list, then converts this to
>> set and then back to list. The aggregation can instead be done directly
>> into a set.
>>
>> (as an offside, it would make more sense for CpuList to have its storage be
>> a set in the first place as duplicate CPU ids don't make sense for it, but
>> that's a separate discussion :-))
>>
>> The integer conversion of the "a-b" pattern can also be condensed into a
>> single map() expression.
>>
>> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
>> ---
>>  rteval/systopology.py | 15 +++++++--------
>>  1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/rteval/systopology.py b/rteval/systopology.py
>> index b2da7bb..2a28f9c 100644
>> --- a/rteval/systopology.py
>> +++ b/rteval/systopology.py
>> @@ -102,20 +102,19 @@ class CpuList:
>>          """ expand a range string into an array of cpu numbers
>>          don't error check against online cpus
>>          """
>> -        result = []
>> -
>>          if not cpulist:
>> -            return result
>> +            return []
>> +
>> +        result = set()
>>
>>          for part in cpulist.split(','):
>>              if '-' in part:
>> -                a, b = part.split('-')
>> -                a, b = int(a), int(b)
>> -                result.extend(list(range(a, b + 1)))
>> +                a, b = map(int, part.split('-'))
>> +                result |= set(range(a, b + 1))
>>              else:
>>                  a = int(part)
>> -                result.append(a)
>> -        return [int(i) for i in list(set(result))]
>> +                result |= {a}
>> +        return list(result)
>>
>>      def getcpulist(self):
>>          """ return the list of cpus tracked """
>> --
>> 2.27.0
>>
>>
>
> I'm guessing that the reason for the "set" was to remove any potential
> duplicates. Duplicates are not normally a problem in rteval, but if you
> want to ensure you handle any user input correctly, it makes sense to do
> that. I think your code is correct, but I'm not sure if it buys us
> anything.
>

It doesn't really, it should be functionaly identical to the existing code
- it's just "drive-by cleanup" really :)

> John