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@xxxxxxxxxx>
> ---
> 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.
John
